[PATCH] D139395: Add CFI integer types normalization

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG71c7313f42d2: Add CFI integer types normalization (authored by rcvalle, committed by samitolvanen). Changed prior to commit:

[PATCH] D139395: Add CFI integer types normalization

samitolvanen accepted this revision. samitolvanen added a comment. Thanks for fixing the MSan issue, Ramon. There's still a clang-format error that trips the Debian build above, but it's trivial so I can fix it when relanding the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 495860. rcvalle added a comment. Fixed initialization order warning Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 495718. rcvalle added a comment. Fixed use of uninitialized value Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst

[PATCH] D139395: Add CFI integer types normalization

hctim added a comment. Hey folks, looks like this caused a failure on the msan buildbot: https://lab.llvm.org/buildbot/#/builders/237/builds/785 It's been had a long-running bug that I'm still tracking down but seems like this is a new failure caused by this patch. The track-origins log is

[PATCH] D139395: Add CFI integer types normalization

This revision was automatically updated to reflect the committed changes. Closed by commit rGb1e9ab7438a0: Add CFI integer types normalization (authored by rcvalle, committed by samitolvanen). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139395: Add CFI integer types normalization

pcc accepted this revision. pcc added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D139395: Add CFI integer types normalization

rcvalle marked 4 inline comments as done. rcvalle added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1694 + getCXXABI().getMangleContext().mangleTypeName( + T, Out, !!getCodeGenOpts().SanitizeCfiICallNormalizeIntegers); + pcc wrote: >

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 493732. rcvalle marked an inline comment as done. rcvalle added a comment. Changed as per review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files:

[PATCH] D139395: Add CFI integer types normalization

pcc accepted this revision. pcc added a comment. LGTM with nits Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1694 + getCXXABI().getMangleContext().mangleTypeName( + T, Out, !!getCodeGenOpts().SanitizeCfiICallNormalizeIntegers); + Is the !! necessary

[PATCH] D139395: Add CFI integer types normalization

samitolvanen added a reviewer: samitolvanen. samitolvanen accepted this revision. samitolvanen added a comment. This revision is now accepted and ready to land. Thanks, LGTM. @pcc, does this version look fine to you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139395: Add CFI integer types normalization

rcvalle marked an inline comment as done. rcvalle added inline comments. Comment at: clang/docs/ControlFlowIntegrity.rst:241 + +``-fsanitize-cfi--icall-experimental-normalize-integers`` +- samitolvanen

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 491013. rcvalle added a comment. Fixed typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst clang/docs/UsersManual.rst

[PATCH] D139395: Add CFI integer types normalization

samitolvanen added a comment. In D139395#4067391 , @pcc wrote: > I discussed this out of band with Ramon and we agreed that the new option > should be marked as experimental because the rustc implementation is not yet > finalized. I think that the

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 490976. rcvalle added a comment. Mark as experimental Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst

[PATCH] D139395: Add CFI integer types normalization

pcc added a comment. In D139395#4066948 , @samitolvanen wrote: > Thanks for the patch, Ramon. This looks like a reasonable approach to me, and > just for reference, here appears to be the corresponding rustc change: > >

[PATCH] D139395: Add CFI integer types normalization

samitolvanen added a comment. Thanks for the patch, Ramon. This looks like a reasonable approach to me, and just for reference, here appears to be the corresponding rustc change: https://github.com/rust-lang/rust/pull/105452/commits/9087c336103d0fa0b465acf8dbc1e4651250fb05 @pcc did you have

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 482708. rcvalle added a comment. Added KCFI support Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 482699. rcvalle added a comment. Updated tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst

[PATCH] D139395: Add CFI integer types normalization

pcc added a comment. In D139395#3990772 , @rcvalle wrote: > I elaborated on the reasons why not use a generalized encoding in the design > document in the tracking issue > https://github.com/rust-lang/rust/issues/89653. The tl;dr; is that it will >

[PATCH] D139395: Add CFI integer types normalization

rcvalle added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:2952 + // uu + if (NormalizeIntegers && T->isInteger()) { +if (T->isSignedInteger()) { pcc wrote: > `isInteger()` will return true for enums, but only if they are complete. This >

[PATCH] D139395: Add CFI integer types normalization

rcvalle added a comment. I elaborated on the reasons why not use a generalized encoding in the design document in the tracking issue https://github.com/rust-lang/rust/issues/89653. The tl;dr; is that it will result in less comprehensive protection by either using a generalized encoding for all

[PATCH] D139395: Add CFI integer types normalization

pcc added a reviewer: pcc. pcc added a comment. A high level question is whether we want to base the cross-language encoding on Itanium at all. Itanium has concepts such as substitutions that will complicate the implementation in other languages. Encoding pointee types can also lead to

[PATCH] D139395: Add CFI integer types normalization

nickdesaulniers added subscribers: hjl.tools, rjmccall. nickdesaulniers added a comment. Thanks for the patch and the work on cross language CFI support! I wonder if we have precedent for other non-standard extensions to `ItaniumMangleContextImpl`? I wonder if we should perhaps have a distinct

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 482327. rcvalle added a comment. Added ".normalized" suffix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 481016. rcvalle added a comment. Updated tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 480775. rcvalle added a comment. Fixed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 480773. rcvalle added a comment. Added compression Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 Files: clang/docs/ControlFlowIntegrity.rst

[PATCH] D139395: Add CFI integer types normalization

rcvalle updated this revision to Diff 480700. rcvalle retitled this revision from "Add support for integer types normalization" to "Add CFI integer types normalization". rcvalle edited the summary of this revision. rcvalle added a comment. Added documentation Repository: rG LLVM Github