4 ms·

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

Tomte 6y ago
I read the SQLite commit such that they weren't aware it's not a released compiler.
SQLite 6y ago
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.
Someone 6y ago
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.
- jfkebwjsbx 6y ago
  Perhaps, but the justification behind the change should be that one, not the miscompilation...
- dathinab 6y ago
  > 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.
  - shawnz 6y ago
    I think what they are saying is, why does sqlite3VdbeMemRelease change the flags in the first place if the change must be ignored?
    - thesz 6y ago
      Because these changes may not be ignored in some other place?
      - Someone 6y ago
        Probably, but it doesn’t look like good API design to provide a function whose actions you sometimes have to partially undo.
RhysU 6y ago
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.
- jfkebwjsbx 6y ago
  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.
  - skykooler 6y ago
    Why is OSSFuzz using an unreleased compiler to test things?
    - the_why_of_y 6y ago
      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.
  - rurban 6y ago
    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.

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.

Someone 6y ago

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.

jfkebwjsbx 6y ago
Perhaps, but the justification behind the change should be that one, not the miscompilation...
dathinab 6y ago
> 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.
- shawnz 6y ago
  I think what they are saying is, why does sqlite3VdbeMemRelease change the flags in the first place if the change must be ignored?
  - thesz 6y ago
    Because these changes may not be ignored in some other place?
    - Someone 6y ago
      Probably, but it doesn’t look like good API design to provide a function whose actions you sometimes have to partially undo.

RhysU 6y ago

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.

jfkebwjsbx 6y ago
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.
- skykooler 6y ago
  Why is OSSFuzz using an unreleased compiler to test things?
  - the_why_of_y 6y ago
    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.
- rurban 6y ago
  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.