Analyzer failure when using &rc[0][0] on register char *rc[];

[llvmbot]

Bugzilla Link	4011
Resolution	FIXED
Resolved on	Feb 22, 2010 12:54
Version	unspecified
OS	All
Reporter	LLVM Bugzilla Contributor
CC	@efriedma-quic,@tkremenek

Extended Description

checker-186.

The code below is specifically meant to test edge cases in the standard. C99 says the code below results in "undefined behavior", though GCC accepts it and does what you'd expect.

The analyzer has some kind of internal failure on the following code:

#include <stdio.h>

int main(int argc, char *argv[]) {
register char *rc[] = {"Hello"};
char *rcp = &rc[0][0];

printf("'%s' = '%s' / sizeof: %ld, %ld\n", rcp, rc[0], sizeof(rc), sizeof(rc[0]));

return(0);
}

shell% ./scan-build gcc regp.c -c -Wall -Wextra -std=gnu99
regp.c:3: warning: unused parameter 'argc'
regp.c:3: warning: unused parameter 'argv'
regp.c:5:15: error: address of register variable requested
char *rcp = &rc[0][0];
^~~~~~~~~
1 diagnostic generated.
scan-build: 0 bugs found.
scan-build: The analyzer encountered problems on some source files.
scan-build: Preprocessed versions of these sources were deposited in '/var/folders/WS/WSIqx8iyG+KqH2i-Y8Yy2E+++TI/-Tmp-/scan-build-2009-04-20-9/failures'.
scan-build: Please consider submitting a bug report using these files:
scan-build: http://clang.llvm.org/StaticAnalysisUsage.html#filingbugs

[efriedma-quic]

The analyzer has some kind of internal failure

To be precise, clang is concluding that the code is invalid while gcc accepts it. In this case, I don't see any reason for clang to accept the given code.

[llvmbot]

I don't think I agree with your assessment Eli. &rc[0][0] is rc[0], which isn't the address of a register variable?

[efriedma-quic]

I don't think I agree with your assessment Eli. &rc[0][0] is rc[0], which isn't
the address of a register variable?

I suppose the relevant code could use some work... clang is incorrectly rejecting the following testcase:
char* a() { register struct {char* x;} x = {"Hello"}; return &(x.x[0]); }

The original testcase isn't valid, though: the expression rc[0] is implicitly taking the address of rc.

[llvmbot]

I looked at this a bit more. The problem isn't really with clang, but more with scan-build.

The original test case is 'undefined behavior' in C99, but the issue isn't that the code is 'illegal', it's that GCC accepts it, but clang punts on it. This seems to confuse scan-build, which I'm sort of guessing at this point has an unstated assumption that if GCC didn't have a problem with it, neither should clang. Then when clang kicks back errors, scan-build prints out a message that 'something bad happened, file a bug'.

Not sure how to deal with this since the assumption is valid the vast majority of the time, but it happens to fail for this corner case.

[llvmbot]

I looked at this some more. The why I picked the initial code is because it really pushes the corner cases. This really requires some time spent with the language spec to make sure the details are right.

There is actually a problem buried in here, but the first report did quite capture it.

Consider the following:

int main(int argc, char *argv[]) {
register char *rc1[1][8] = {{"Hello"}};
char *rc1p = rc1[0][0];

register char *rc2[] = {"Hello"};
char *rc2p = rc2[0];
char *rc2ap = &rc2[0][0];
}

shell% bin/clang regp.c
regp.c:11:17: error: address of register variable requested
char *rc2ap = &rc2[0][0];
^~~~~~~~~~
1 diagnostic generated.
shell%

Only one error is generated, even though rc2p and rc2ap would contain the same pointer result (provided the address-of operator succeeded). The 'right thing' to do in this case is kinda iffy, which is why I'm sure it's left as undefined behavior. 6.5.3.2 clause 1 states:

The operand of the unary & operator shall be either a function designator, the result of a
[] or unary * operator, or an lvalue that designates an object that is not a bit-field and is
not declared with the register storage-class specifier.

Eli offered the following:

char* a() { register struct {char* x;} x = {"Hello"}; return &(x.x[0]); }

This example is really no different than that one I put forth. Specifically, C99 6.7.1 clause 6 states:

If an aggregate or union object is declared with a storage-class specifier other than
typedef, the properties resulting from the storage-class specifier, except with respect to
linkage, also apply to the members of the object, and so on recursively for any aggregate
or union member objects.

Now consider the following:

int main(int argc, char *argp[]) {
register int x[10] = {0,1,2,3,4,5,6,7,8,9};
int *xp = x;
return(0);
}
shell% bin/clang regp.c
shell%

No error is generated.

6.3.2.1 clause 3 states:

Except when it is the operand of the sizeof operator or the unary & operator, or is a
string literal used to initialize an array, an expression that has type "array of type" is
converted to an expression with type "pointer to type" that points to the initial element of
the array object and is not an lvalue. If the array object has register storage class, the
behavior is undefined.

[efriedma-quic]

The original test case is 'undefined behavior' in C99, but the issue isn't that
the code is 'illegal', it's that GCC accepts it, but clang punts on it. This
seems to confuse scan-build, which I'm sort of guessing at this point has an
unstated assumption that if GCC didn't have a problem with it, neither should
clang. Then when clang kicks back errors, scan-build prints out a message that
'something bad happened, file a bug'.

We should probably change the message to say something more like "clang thinks this code isn't valid (see diagnostics above). If you think it is in fact valid, please check the bug database at llvm.org/bugs/, and file a bug if there isn't already one filed." The original wording was written back when an inconsistency was much more likely to be a clang bug.

[efriedma-quic]

Only one error is generated, even though rc2p and rc2ap would contain the same
pointer result (provided the address-of operator succeeded). The 'right thing'
to do in this case is kinda iffy, which is why I'm sure it's left as undefined
behavior.

The right thing to do is print an error; it's a missing diagnostic.

r69586 cleans things up a bit in this area, so we should be more consistent.

[tkremenek]

The original test case is 'undefined behavior' in C99, but the issue isn't that
the code is 'illegal', it's that GCC accepts it, but clang punts on it. This
seems to confuse scan-build, which I'm sort of guessing at this point has an
unstated assumption that if GCC didn't have a problem with it, neither should
clang. Then when clang kicks back errors, scan-build prints out a message that
'something bad happened, file a bug'.

We should probably change the message to say something more like "clang thinks
this code isn't valid (see diagnostics above). If you think it is in fact
valid, please check the bug database at llvm.org/bugs/, and file a bug if there
isn't already one filed." The original wording was written back when an
inconsistency was much more likely to be a clang bug.

Now that codegen has become a mature client of the frontend, it is probably better for the analyzer not even to emit this warnings at all (unless clang-cc crashed). Clients who try and build a project will quickly see if there is a build error.

I've changed ccc-analyzer to not emit "Parser Rejects" warnings by default:

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20090427/016405.html

[tkremenek]

I've changed ccc-analyzer to not emit "Parser Rejects" warnings by default:

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20090427/016405.html

If this is a satisfactory fix, should this bug be marked fixed?