Hacker News Reader

Comment by jfkebwjsbx | Hacker News Reader

What strikes me as odd is why SQLite is committing changes just to make an unreleased compiler happy...

It was late at night. I didn't realize the clang-11.0.0 was unreleased until I saw this HN thread the next morning. The only reason I was using clang-11.0.0 is because the OSSFuzz bug report against SQLite used clang-11.0.0 and I couldn't get the (alleged) SQLite problem to repro using any other compiler.

I think it’s a code improvement, too. It now computes the new flags in one place.

Just looking at these lines, I don’t understand the code, though. Apparently, sqlite3VdbeMemRelease can change pMem->flags, but that change can be ignored. I can’t think of a simple explanation for that.

Also, the function name sqlite3VdbeMemRelease seems to indicate memory gets released, but pMem->flags = MEM_Str|MEM_Term…, to me, indicates it contains a string.

Perhaps, but the justification behind the change should be that one, not the miscompilation...

> but that change can be ignore

No the changes _must_ be ignored. This is where the bug came from.

The code does store the flags, does an operation and then restores some flags while setting some new ones.

But after clang reorders the operations it will not restore the flags from _before_ the function calls but instead takes them from _after_ the function call.

I.e. any change `sqlite3VdbeMemRelease` does to `pMem` mutst be ignored but after reordering changes to `MEM_AffMask` `MEM_Subtype` are no longer ignored.

I think what they are saying is, why does sqlite3VdbeMemRelease change the flags in the first place if the change must be ignored?

Because these changes may not be ignored in some other place?

Probably, but it doesn’t look like good API design to provide a function whose actions you sometimes have to partially undo.

SQLite is so pervasive that when, not, if someone compiles it with the busted compiler (or others with a similar mistake) they want to be ready.

The compiler is unreleased and months away. There are many, many broken revisions of compilers in development.

A project cannot burden its code trying to work on every single commit of every compiler out there.

Why is OSSFuzz using an unreleased compiler to test things?

Likely because the company that provides OSSFuzz employs a large number of developers that work on the unreleased compiler, so consider it part of the QA process.

But not with gcc, which releases broken versions since 9.0 and cannot catch up fixing these regressions, mostly in the str library. Just look at their Bugzilla.

clang fixed it in one day. for gcc I expect 5 years to fix their regressions.

I read the SQLite commit such that they weren't aware it's not a released compiler.

Reply on news.ycombinator.com