Well, I just looked in the 3.7.2 amalgamation version I use.
1) sqlite3SrcListShiftJoinType was declared with SQLITE_PRIVATE. Even
if the static isn't here, if it is in the prototype, it can apply.
Since this is an amalgamation-only feature, this might not be the
issue. (3.7.7.1 src did not have SQLITE_PRIVATE, but 3.7.7.1
amalgamation did.)
2) Member "a" was an array, not a pointer. Arrays cannot be null.
This was the same in 3.7.7.1. Array members can only be null if:
A) they are the first member of the structure and the pointer to
the structure is null. In this case, it is not the first
member, and you tested the pointer to the structure in any case.
B) You wrap-around memory, which is undefined behavior.
In short, GCC 4.1.0 got it RIGHT. GCC 4.5.2 optimized less. I
suspect that either the default optimization has changed or the level
for this optimization has changed.
Having said this, it may well be reasonable to leave the test in place
in case you someday choose to stop using the struct hack (which is
undefined behavior as well), knowing that the compiler will optimize
out the dead test.
--David Garfield
Richard Hipp writes:
> Consider this line of code in the "build.c" source file of SQLite:
>
> http://www.sqlite.org/src/artifact/77be7c217430?ln=3372
>
> It appears that GCC 4.1.0 is not generating any code for the second test in
> the conditional. In other words, GCC 4.1.0 is compiling that statement as
> if it omitted the "&& p->a" term and looked like this:
>
> if( p ){ ....
>
> You can see this for yourself by downloading the file above and then
> running:
>
> gcc -g -S build.c
>
> And then looking at the build.s output file. With GCC 4.1.0, I get this:
>
> .loc 1 3372 0
> cmpl $0, 8(%ebp)
> je .L920
>
> Looks like only one test to me. But with GCC 4.5.2 I get this:
>
> .loc 1 3372 0
> cmpq $0, -24(%rbp)
> je .L611
> .loc 1 3372 0 is_stmt 0 discriminator 1
> movq -24(%rbp), %rax
> addq $8, %rax
> testq %rax, %rax
> je .L611
>
> Both tests appear to be coded this time.
>
> As it happens, the GCC bug is harmless in this case. SQLite never invokes
> the sqlite3SrcListShiftJoinType() function with a non-NULL SrcList pointer
> that has a NULL p->a value. So the p->a!=NULL test really is always true.
> (Note that the GCC optimizer has no way of knowing that because the function
> has external linkage.) And so it didn't matter that the test was omitted.
> I didn't notice the problem until this morning, when I upgraded my desktop
> to the latest Ubuntu containing GCC 4.5.2, and reran the full branch
> coverage tests. GCC 4.5.2 was showing that the p->a!=NULL branch was always
> true. Further investigation shows that it has always been always true but
> that the GCC 4.1.0 bug simply masked the error up until now.
>
> I see two take-aways from this episode:
>
> (1) Compilers sometimes make mistakes. So it is important that you test
> your object code - not just your source code. That means running your test
> cases using exactly the same *.o files that you use for delivery. "Fly what
> you test and test what you fly."
>
> (2) I need to come up with a second, independent method of verifying branch
> test coverage in SQLite. I have been using GCC+GCOV and it does a great job
> and I fully intend to continue using it as the primary tool chain for
> development and testing. But in this case, because GCC was omitting a test,
> it missed the fact that there was no test coverage for the omitted test. So
> it would be nice to have an independently developed tool chain that can be
> used to confirm the results we get from GCOV. Anybody have any suggestions?
>
> --
> D. Richard Hipp
> d...@sqlite.org
> _______________________________________________
> sqlite-users mailing list
> sqlite-users@sqlite.org
> http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
_______________________________________________
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
