[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

yihanaa added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:433 + + llvm::StringRef getFormatSpecifier(QualType T) { +if (auto *BT = T->getAs()) { erichkeane wrote: > rsmith wrote: > > yihanaa wrote: > > > rsmith wrote: > > > > rsmith wrote: >

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

yihanaa added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:433 + + llvm::StringRef getFormatSpecifier(QualType T) { +if (auto *BT = T->getAs()) { yihanaa wrote: > erichkeane wrote: > > rsmith wrote: > > > yihanaa wrote: > > > > rsmith wrote:

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

yihanaa added a comment. I found LLDB have this feature. LLDB uses "..." to indicate that only part of the string is now printed, and it also have a command to modify length limit. This approach is similar to the idea of @erichkeane, can we do the same？what do you all think? typedef signed

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

yihanaa added a comment. This is unfortunately true, unless we generate some code that calculates the length of the string at runtime. I find it difficult to draw the line between defining this built-in functionality and developers developing printf-like functionality, if built-in does too much

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

yihanaa created this revision. yihanaa added reviewers: rjmccall, aaron.ballman, erichkeane, lebedev.ri. yihanaa added a project: clang. Herald added a project: All. yihanaa requested review of this revision. Herald added a subscriber: cfe-commits. Clang will crash when __builtin_assume_aligned's

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

yihanaa added a comment. In D131979#3726686 , @erichkeane wrote: > I think the correct answer here is instead to prohibit using this on > array/string literal types in Sema instead of just ignoring the call. Thanks for you replay @erichkeane , in clan

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

yihanaa added a comment. Thanks for your suggestion @erichkeane @rjmccall , I will try to do that Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131979/new/ https://reviews.llvm.org/D131979 ___ cfe-commit

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

yihanaa added a comment. As far as I know, turning on the -fsanitizer=alignment options when calling __builtin_assume_aligned in C code, if the 1st arg has volatile qualifier, Clang will emit "call void @__ubsan_handle_alignment_assumption(...)" in CodeGen, else Clang will emit an warning and ig

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

yihanaa added a comment. In D131979#3743127 , @rjmccall wrote: > In D131979#3742399 , @yihanaa wrote: > >> As far as I know, turning on the -fsanitizer=alignment options when calling >> __builtin_assume_aligned i

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

yihanaa updated this revision to Diff 455918. yihanaa added a comment. Thanks John and Erich for your comments. In this update: Check args in Sema, and both in C and C++, emit an error if the 1st arg of __builtin_assume_aligned is volatile qualified or 1st arg not a pointer type. Remove getSubE

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

yihanaa added a comment. In D131979#3752131 , @rjmccall wrote: > I'm a little worried about the shift to emit an unconditional error on > `volatile`. Can we at least separate that into its own patch and just fix > the bug in this one? And please try t

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

yihanaa added a comment. In D131979#3752208 , @rjmccall wrote: > From the test case, it looks like the builtin just ignores pointers to > volatile types, which should be preserved by the conversions you're now doing > in Sema. That is, you should be ab

<    1   2