Re: JSON/YAML/TOML/etc. parsing performance

[Paul Eggert]

On 10/05/2017 12:12 AM, Eli Zaretskii wrote:
> If ptrdiff_t overflows are reliably diagnosed, then why do we have to
> test for them explicitly in our code, as in the proposed json.c?
They're diagnosed only if one compiles with debugging flags like 
-fsanitize=undefined. And even then the checks are "reliable" only in 
some sense: some overflows at the source level are not caught at the 
machine level even if the code is executed, because the overflows are 
optimized away. So testing a program with -fsanizitize=undefined does 
not guarantee that the same test will avoid ptrdiff_t overflow on some 
other platform.

> AFAIU, ptrdiff_t overflows are the _only_ reason for json.c checks
> whether a size_t value is too large

In the most recent patch I proposed, there were only two such checks; 
there were two other checks for too-large size_t that were fixnum 
checks, not ptrdiff_t checks.

However, I can see that you really don't like those checks. So I tweaked 
the proposed patch to remove them all from json.c. Please see the 
attached 3 patches (the first is just Philipp's patch rebased to 
master). The basic idea here is that json.c should be using xmalloc for 
allocation anyway, for reasons other than size overflow checking. And 
once it uses the Emacs malloc we can make sure that it never allocates 
objects that are too large for ptrdiff_t.

> I'm not arguing for general replacement of ptrdiff_t with size_t, only
> for doing that in those primitives where negative values are a clear
> mistake/bug.

This is exactly where we should be cautious about using unsigned types. 
The longstanding Emacs style is to prefer signed integers, to avoid the 
common typos that occur with unsigned. If we start changing the style, 
these primitives (or people debugging these primitives) often won't be 
able to distinguish buggy from valid-but-enormous cases. And when we 
compile such primitives (or their callers) with -fsanitize=undefined, we 
won't be able to catch integer-overflow bugs automatically at runtime, 
since unsigned integer arithmetic silently wraps around even when 
-fsanitize=undefined is used.

> "kids, don't do that at home -- we are trained professionals" 

I help maintain several GNU programs that use unsigned types for sizes, 
and find that style to be trickier than the style Emacs uses, with 
respect to integer-overflow bugs. I've been gradually changing some of 
the non-Emacs GNU code to use signed types, and the results have been 
encouraging: the code is more readable and more obviously correct. I 
would rather not go back to the unsigned style: although you're right 
that it can be done, it is too error-prone and the errors can lead to 
serious bugs. Even for trained professionals, this particular set of 
acrobatics is best done with a net.

0001-Implement-native-JSON-support-using-Jansson.patch
Description: Text Data

0002-Do-not-malloc-more-than-PTRDIFF_MAX.patch
Description: Text Data

0003-Minor-JSON-cleanups-mostly-for-overflow.patch
Description: Text Data