An Experiment with Checking the glibc Library | Hacker News Reader

An Experiment with Checking the glibc Library

viva64.com

19 pointsAndreyKarpov11 comments

I'm not buying these conclusions - on a quick scan, the "strange expression" isn't wrong: the code is scanning to find the first character that isn't a colon. The check against the zero byte is harmless, and I could imagine other static analysis tools insisting on it (as cheap insurance against scanning off-the-end).

As for the "shifting negative integers is undefined behavior", this is a technically-true statement that's been practically-false for quite a few years - does anybody still use a machine that doesn't use two's-complement integers?

The strange expression does seem wrong to me: the loop should terminate on end-of-line or ":", so continue while not end-of-line and not ":".

I think you're right that shifting negative integers is fine on most any platform you're likely to encounter, but until recently one could also assume that adding two signed integers would wrap in practice, despite being undefined in theory; due to gcc optimizing ever more aggressively, code assuming that signed integers wrap is now broken (see e.g. http://thiemonagel.de/2010/01/signed-integer-overflow/). So I'd be careful with shifts of negative integers in my own code.

It's skipping over ':' characters (which serve as delimiters) to get to the next token of the string (or end of line). So it looks correct, but the check for end of line is superfluous (unless any architecture uses ':' as end of line marker of course).

I think it is more likely it looks the way it does because it was made by copy-pasting and editing the loop a few lines further down that copies a token:

  while (categoryvalue[0] != '\0' && categoryvalue[0] != ':')
    *cp++ = *categoryvalue++;

As for the shifting, I would assume glibc is free to expect whatever holds true on the platforms it currently supports. But fair enough warning about it in a analysis tool.

The compiler is allowed to remove the call of the memset() function when compiling the Release version. Well, it is actually obliged to do so for the sake of optimization. Since the 'temp_result' buffer is not used anywhere after calling the memset() function, the function call itself is not needed too.

I'm constantly amazed by the perversity of 'compliant' compilers, but can this be true? It seems really unlikely unless memset() is treated specially in the spec.

Presumably a compiler is not 'obliged' to remove calls to write() or free() just because the argument is not used again after the call. How can it tell the difference?

I've always presumed that if I call a function in a shared library, that the call will be made. Is this not the case?

Well, I would guess that since memset() is a well-known standard library function that actually has a specification, and its buffer argument is not volatile, then the compiler can "get away" with assuming things.

It's the same for e.g. strcmp() and strcpy(), they can be compiled into specialized instructions when the compiler considers that a win.

GCC's set of builtins is quite large: http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html.

To comment on the (very interesting) article, I think this:

    XDR *xdrs = &clp->xdr_stream;

is not at all an error, even if clp is NULL. No de-referencing occurs, since the expression is just the argument of the & (address-of, in C) operator. See the traditional offsetof() definition: http://en.wikipedia.org/wiki/Offsetof which explicitly does this for a base pointer of 0.

The reason why this is allowed is that is seemingly pointless code. After this buffer is memset'd it is never referenced again and falls out of scope. Assuming that all other code behaves safe and doesn't rely on undefined behavior it is unreachable. It doesn't matter if you set it to zero or not if no one reads it. For a lot of code this is a valid assumption and a safe optimizations. Read and write calls are special since they do have side effects that can't be optimized out by a compiler. Someone please correct me if I am misunderstanding this code snippet.

I am fine if a compiler makes an optimization like this for my code but I can understand why this is unacceptable for crypto.

> I'm constantly amazed by the perversity of 'compliant' compilers, but can this be true?

V597 - http://www.viva64.com/en/d/0208/

Zero and forget -- caveats of zeroing memory in C - http://www.eliteraspberries.com/blog/2012/10/zero-and-forget...

Overwriting memory - why? - http://www.viva64.com/en/k/0041/

I think I understand those issues, and I agree that this is potentially a real bug. It's the statements in your description I'm not sure I agree with.

I do not believe though that a compiler is allowed to remove a function call to the external library function call memset(), as it does not know whether there are any side effects to that call. It does not matter if I use the argument again afterward, a library function call will not be optimized out. Consider: what if at runtime I link to a different library where memset() performs IO?

The issue is that there is that there may be no function call to the library function memset() in the source. Rather, memset() may be defined as a macro such that the memory manipulations are inlined. Since the optimizer sees only these memory operations (rather than the function call), in the absence of a 'volatile' declaration it can legally delete them.

Thus whether this is actually a bug depends on whether the compiler treats memset() as a builtin macro. This is only a bug if there is no function call in the source by the time the preprocessor has done its work. One does not have to live in fear that a compliant compiler will delete non-local function calls. Or at least if it does, it's a bug in the compiler.

>One might expect the glibc library's code to be written in such a way as to provide maximum portability.

Its worth noting portability was one of the reasons cited by Debian for moving from glibc to eglibc.

I am shocked to see the strlen expansion. Is it common or something specific for some very special compiler options? I can't imagine a lot of cases where I would want that in my code.

Looks like yet another dependency that I'll have to build from source code, and test, fix, and refactor myself.

Reply on news.ycombinator.com