[PATCH] D33305: [ubsan] Add a check for pointer overflow UB

sberg added a comment. Just a heads up that I ran into an arguably somewhat unexpected instance of this with (a copy of the Graphite project included in) LibreOffice, see the commit message of https://cgit.freedesktop.org/libreoffice/core/commit/?id=681b4a49d797996229513d3e842d2a431030730a "ex

[PATCH] D33030: [libcxxabi] Align unwindHeader on a double-word boundary

sberg added a comment. In LibreOffice we unfortunately have some dirty code that synthesizes exception throwing, calling `__cxa_allocate_exception`, `__cxa_throw` (passing in our own exception destructor function), and then, when the exception destructor function gets called, assumes we can map

[PATCH] D34417: Switch TestVisitor to Lang_C via -x c

sberg created this revision. ...instead of -std=c99, as the latter lead to error: invalid argument '-std=c99' not allowed with 'C++' complaints in test logs https://reviews.llvm.org/D34417 Files: unittests/Tooling/TestVisitor.h Index: unittests/Tooling/TestVisitor.h

[PATCH] D34419: Make sure TraverseInitListExpr visits InitListExpr exactly twice

sberg created this revision. ... once each for the syntactic and semantic form. Without this fix, behavior of the newly added tests would have been InitListExprIsPreOrderVisitedTwice: syntactic: 1 semantic: 2 InitListExprIsPostOrderVisitedTwice: syntactic: 0 semantic: 1 InitListExprIsPre

[PATCH] D22128: Make CastExpr::getSubExprAsWritten look through implicit temporary under CK_ConstructorConversion

sberg updated this revision to Diff 103449. sberg added a comment. Herald added subscribers: kristof.beyls, mgorny, klimek. added test https://reviews.llvm.org/D22128 Files: lib/AST/Expr.cpp unittests/Tooling/CMakeLists.txt unittests/Tooling/CastExprTest.cpp Index: unittests/Tooling/Cast

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC320978: No -fsanitize=function warning when calling noexcept function through non… (authored by sberg, committed by ). Ch

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg reopened this revision. sberg added a comment. Had to revert r320977/r320978 again with r320981/r320982: "At least http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/6013/steps/annotate/logs/stdio complains about ubsan::ubsan_handle_function_type_mismatch_abort (compi

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg added a comment. In https://reviews.llvm.org/D40720#958677, @vsk wrote: > Please add a test. Note that the bot upon the first closing of this review changed the shown diff from the combined cfe+compiler-rt diff to just the cfe part. See https://reviews.llvm.org/rL320977 for the compile

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg added a comment. In https://reviews.llvm.org/D40720#958743, @vsk wrote: > > Would it be possible to fix this by stripping the noexcept specifiers from > > both the function type used in the check and the one that is embedded in > > the prefix data? The downside is that we won't catch the

[PATCH] D40295: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid

sberg updated this revision to Diff 127676. sberg added a comment. added a small IR test https://reviews.llvm.org/D40295 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGExprCXX.cpp clang/lib/CodeGen/CodeGenFunction.h clang/test/CodeGenCXX/ubsan-vtable-checks.cpp compiler-rt/li

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg updated this revision to Diff 127863. sberg added a comment. As suggested, solve the issue instead by removing any "noexcept" from the typeinfo emitted for the -fsanitize=function checks. https://reviews.llvm.org/D40720 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFun

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg updated this revision to Diff 127899. sberg added a comment. (need to call getAs instead of cast in one place, in case the name in the function decl is wrapped in parens, as happens in HarfBuzz's hb-buffer.cc) https://reviews.llvm.org/D40720 Files: clang/lib/CodeGen/CGExpr.cpp clang

[PATCH] D40295: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid

This revision was automatically updated to reflect the committed changes. Closed by commit rL321518: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid (authored by sberg, committed by ). Changed prior to commit: https://reviews.llvm.org/D40295?vs=127676&id=128288#toc Repo

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg updated this revision to Diff 128289. sberg added a comment. made the recommended changes https://reviews.llvm.org/D40720 Files: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/CodeGen

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg added a comment. friendly ping any further input, or should I consider this good enough to go in now? https://reviews.llvm.org/D40720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

This revision was automatically updated to reflect the committed changes. Closed by commit rC321859: No -fsanitize=function warning when calling noexcept function through non… (authored by sberg, committed by ). Changed prior to commit: https://reviews.llvm.org/D40720?vs=128289&id=128710#toc R

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg added a comment. Should this be backported to Clang 6? Not sure how widespread a problem this is in practice (it hit me with LibreOffice). Repository: rC Clang https://reviews.llvm.org/D40720 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D16632: clang-cl: Take dllexport from original function decl into account

sberg updated this revision to Diff 137374. sberg added a comment. Turns out DLLAttr-inherited-from-class is only added to members during Sema::CheckCompletedClass -> Sema::checkClassLevelDLLAttribute, when friend re-decls of those members may already have been created. https://reviews.llvm.or

[PATCH] D16632: clang-cl: Take dllexport from original function decl into account

This revision was automatically updated to reflect the committed changes. Closed by commit rL326990: Propagate DLLAttr to friend re-declarations of member functions (authored by sberg, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D16

[PATCH] D52400: Improve -Wshadow warnings with enumerators

sberg added a comment. doesnt this make -Wshadow more aggressive for enumerators than for other entities? ~ cat test17.cc struct S1; struct S2; struct S3 { void S1(); enum { S2 }; }; ~ llvm/inst/bin/clang++ -fsyntax-only -Wshadow test17.cc test17.cc:5:10: warning: decla

[PATCH] D52400: Improve -Wshadow warnings with enumerators

sberg added a comment. In https://reviews.llvm.org/D52400#1266341, @aaron.ballman wrote: > In https://reviews.llvm.org/D52400#1266307, @sberg wrote: > > > > [...] > Then again, this is a case where you don't get any error but you do get a > silent behavioral ambiguity without the current enum

[PATCH] D52400: Improve -Wshadow warnings with enumerators

sberg added a comment. > I've silenced this scenario in r344898, thank you for raising the issue! thanks! works fine for me https://reviews.llvm.org/D52400 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

sberg added a comment. One problem I found with the macro `__cpp_impl_destroying_delete` not being conditional on language version is the following: Recent GCC trunk (since https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=76b94d4ba654e9af1882865933343d11f5c3b18b "Implement P0722R3, destroying oper

[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

sberg added a comment. > I run the new checker on LibreOffice project. I found ~25 false positives, > which seems small enough to me. This false positives can be supressed easily. Do you have a link to such a false positive and how it got suppressed in the LibreOffice code base? (If those are

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg created this revision. sberg added reviewers: filcab, marxin, rsmith. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, kristof.beyls, javed.absar, kubamracek. Herald added projects: clang, Sanitizers, LLVM. This follows up after b7692bc3e9ad2691fc07261904b88fb15f30696b

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg marked an inline comment as done. sberg added inline comments. Comment at: compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc:264 + return SANITIZER_NON_UNIQUE_TYPEINFO && TI1->__type_name[0] != '*' && + TI2->__type_name[0] != '*' && + !internal_strcmp(TI1->_

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg added a comment. friendly ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60760/new/ https://reviews.llvm.org/D60760 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL359759: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO (authored by sberg, committed by ). Herald added a subs

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg added a comment. In D60760#1487342 , @lebedev.ri wrote: > Did this get reviewed? I didn't get any responses at all, so decided to push it anyway. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60760/new/ https://rev

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

sberg added a comment. Added missing tests at https://reviews.llvm.org/D61479 "Add tests for 'Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO'". Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60760/new/ https://reviews.llvm.org/D60760 ___

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

sberg created this revision. sberg added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://bugs.llvm.org/show_bug.cgi?id=40658 Repository: rC Clang https://reviews.llvm.org/D58056 Files: clang/lib/AST/ASTContext.cpp clang/test/AST/fu

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

sberg added a comment. The change itself should probably be uncontroversial (the bad cast had been there ever since getFunctionTypeWithExceptionSpec had been introduced with r221918), but I'm not sure about the test: It tests the relevant code somewhat indirectly; is it fine in clang/test/AST/?

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC353931: Look through typedefs in getFunctionTypeWithExceptionSpec (authored by sberg, committed by ). Changed prior to co

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

sberg added a comment. committed for now to get the crash fixed; if there are issues with the test they can be addressed later Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58056/new/ https://reviews.llvm.org/D58056 ___

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

sberg updated this revision to Diff 209865. sberg added a comment. - Disallowed -fsanitize=function in combination with -fsanitize-minimal-runtime now. - Left the ubsan test's RUN lines and \#include magic as-is, if there's no clearly better way to avoid this unfortunate complexity. CHANGES S

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

This revision was automatically updated to reflect the committed changes. Closed by commit rL366186: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO" (authored by sberg, committed by ). Herald added a subscriber: delcypher. Changed prior to commit: https://reviews.llvm.org/D6

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

sberg added a comment. eh, the summary here doesn't get updated from the actual git commit message :( thanks for the reviews! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61479/new/ https://reviews.llvm.org/D61479 _

[PATCH] D66350: Rudimentary support for Doxygen \retval command

sberg created this revision. sberg added a reviewer: gribozavr. sberg added a project: clang. Herald added a subscriber: cfe-commits. ...so that at least a preceding \param etc. that lacks a description gets a -Wdocumentation warning (instead of erroneously treating the \retval ... text as its p

[PATCH] D66350: Rudimentary support for Doxygen \retval command

This revision was automatically updated to reflect the committed changes. Closed by commit rL369345: Rudimentary support for Doxygen \retval command (authored by sberg, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

sberg added a subscriber: cfe-commits. sberg added a comment. Any thoughts on this? (cfe-commits had inadvertently been missing from subscribers, it touches clang as well as compiler-rt.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61479/new/ https://reviews.llvm.org/D61479 _

[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

sberg added a comment. It looks like this caused a clang-cl regression https://bugs.llvm.org/show_bug.cgi?id=37146 "clang-cl emits special functions for non-trivial C-structs ('__destructor_8') introduced for Objective-C". Repository: rL LLVM https://reviews.llvm.org/D41228 _

[PATCH] D45601: Warn on bool* to bool conversion

sberg added a comment. A random data point from trying this patch on the LibreOffice code base: - a little over 100 cases that are easily identified as false positives (many of the form "if (p) *p = ...") - two or three cases that looked suspicious on first glance but turned out to be false po

[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes

sberg added a comment. see https://bugs.llvm.org/show_bug.cgi?id=37161 "clang-cl triggers ASTContext::getASTRecordLayout Assertion `D && 'Cannot get layout of forward declarations!''" for what appears to be fallout from this change Repository: rL LLVM https://reviews.llvm.org/D45112

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

sberg created this revision. sberg added reviewers: aaron.ballman, rsmith, Nathan-Huckleberry. Herald added a project: clang. For a `DeclStmt` like [[maybe_unused]] int i; `getBeginLoc()` returned the start of `int` instead of the start of `[[` (see http://lists.llvm.org/pipermail/cfe-dev/201

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

sberg updated this revision to Diff 224303. sberg edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68581/new/ https://reviews.llvm.org/D68581 Files: clang/lib/Parse/ParseStmt.cpp clang/test/AST/sourceranges.cpp Index: clang/test/AST/sourceranges.cp

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

sberg updated this revision to Diff 224823. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68581/new/ https://reviews.llvm.org/D68581 Files: clang/lib/Parse/ParseStmt.cpp clang/test/AST/sourceranges.cpp Index: clang/test/AST/sourceranges.cpp =

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

This revision was automatically updated to reflect the committed changes. Closed by commit rGdc3957ec215d: Include leading attributes in DeclStmt's SourceRange (authored by sberg). Changed prior to commit: https://reviews.llvm.org/D68581?vs=224823&id=225397#toc Repository: rG LLVM Github Mon

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

sberg marked 2 inline comments as done. sberg added inline comments. Comment at: clang/test/AST/sourceranges.cpp:155 + +#if __cplusplus >= 201703L + void cpp17() { aaron.ballman wrote: > Why is this guarded on C++17? `[[maybe_unused]]` is supported in every > l

[PATCH] D76646: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

sberg added inline comments. Comment at: clang/include/clang/AST/Expr.h:513 - /// isIntegerConstantExpr - Return true if this expression is a valid integer - /// constant expression, and, if so, return its value in Result. If not a - /// valid i-c-e, return false and fill i

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

sberg created this revision. sberg added reviewers: jordan_rose, rtrieu. Herald added a project: clang. sberg requested review of this revision. ...to C++ constant expression operands. (I had been puzzled why Clang had not found

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

sberg updated this revision to Diff 283204. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85287/new/ https://reviews.llvm.org/D85287 Files: clang/lib/Analysis/CFG.cpp clang/test/Sema/warn-bitwise-compare.c clang/test/SemaCXX/warn-bitwise-compare.cpp Index: clang/test/SemaCXX/warn-

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

sberg added a comment. My naive assumption was that this warning had initially been restricted to integer literals and enumerators to avoid false positives. Hence my conservative approach of extending merely to constant integer expressions for now. Maybe Richard as the original author of the

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

sberg added a comment. In D85287#2199463 , @rtrieu wrote: > Are you planning to allow this change to other warnings that use the same > helper functions? No, I didn't plan to work on this further. Just scratching that itch of why Clang didn't emit that

[PATCH] D68720: Support -fstack-clash-protection for x86

sberg added inline comments. Herald added a subscriber: dang. Comment at: clang/include/clang/Driver/Options.td:1778 + HelpText<"Enable stack clash protection">; +def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, Group, + HelpText<"Disable stack clash p

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

sberg added a comment. I think this generates a false positive with `test.cc` enum E { E1 = 1, E2 = 2 }; bool f(E e) { return ((e & E1) ? 1 : 0) + ((e & E2) ? 1 : 0) > 1; } and `clang++ -fsyntax-only -Wtautological-value-range-compare test.cc` test.cc:2:62: warning: result of comparison o

[PATCH] D85778: More accurately compute the ranges of possible values for +, -, *, &, %.

sberg added a comment. Fixes all the false positives it had reported for LibreOffice (which had all involved expressions containing either ~ or +). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85778/new/ https://reviews.llvm.org/D85778 _

[PATCH] D68720: Support -fstack-clash-protection for x86

sberg added a comment. I filed https://bugs.llvm.org/show_bug.cgi?id=47139 "Misspelled -fnostack-clash-protection" now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 _

[PATCH] D40295: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid

sberg updated this revision to Diff 124154. sberg added a comment. Oh, I'd meant to upload the version of the patch where the newly added calls to EmitTypeCheck use the default Alignment and SkippedChecks arguments, instead of explicitly skipping Alignment and ObjectSize checks. (That's the ve

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg created this revision. Herald added a subscriber: kubamracek. As discussed in the mail thread https://groups.google.com/a/isocpp.org/forum/#!topic/std-discussion/T64_dW3WKUk "Calling noexcept function throug non-noexcept pointer is undefined behavior?", such a call should not be UB. Howe

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg updated this revision to Diff 125152. sberg added a comment. (Diff 125121 had accidentally contained a spurious "}". Fixed that now.) https://reviews.llvm.org/D40720 Files: clang/lib/CodeGen/CGExpr.cpp compiler-rt/lib/ubsan/ubsan_handlers.cc compiler-rt/lib/ubsan/ubsan_handlers.h

[PATCH] D40167: In stdbool.h, define bool, false, true only in gnu++98

This revision was automatically updated to reflect the committed changes. Closed by commit rC320135: In stdbool.h, define bool, false, true only in gnu++98 (authored by sberg). Changed prior to commit: https://reviews.llvm.org/D40167?vs=123296&id=126095#toc Repository: rC Clang https://revi

[PATCH] D40295: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid

sberg added a comment. friendly ping; any input on my two questions maybe? https://reviews.llvm.org/D40295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

sberg added a comment. friendly ping https://reviews.llvm.org/D40720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. In D126864#3698798 , @sberg wrote: > I'm surprised that [...] > causes a warning? I would have expected it to be suppressed in this case, as > with the lax `-fstrict-flex-arrays=0` default, and only to hit with the > stricter `

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. see https://reviews.llvm.org/D128643 "[clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible" for another regression CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126864/new/ https://reviews.llvm.org/D126864 ___

[PATCH] D128643: [clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible

sberg created this revision. sberg added a reviewer: serge-sans-paille. sberg added a project: clang. Herald added a project: All. sberg requested review of this revision. ...even if the size resulted from a macro expansion. This reverts back to the behavior prior to https://github.com/llvm/llvm

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. In D126864#3612149 , @sberg wrote: > see https://reviews.llvm.org/D128643 "[clang] -fsanitize=array-bounds: treat > all trailing size-1 arrays as flexible" for another regression For the record (now that the original commit has be

[PATCH] D128643: [clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible

sberg abandoned this revision. sberg added a comment. I'm abandoning this as the underlying https://reviews.llvm.org/D126864 "[clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays" has been reverted for now. I documented all my issues with that underlying commit (incl

[PATCH] D128783: Check for more -fsanitize=array-bounds regressions

sberg created this revision. sberg added a reviewer: MaskRay. sberg added a project: clang. Herald added a subscriber: StephenFan. Herald added a project: All. sberg requested review of this revision. ...that had been introduced with (since reverted) https://github.com/llvm/llvm-project/commit/88

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. In D126864#3614297 , @MaskRay wrote: > Oh, I did not see this when pushing a test > efd90ffbfc427ad4c4675ac1fcae9d53cc7f1322 > . > Consider adding additional t

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

sberg updated this revision to Diff 441313. sberg added a comment. Updated the prospective git commit message as follow: [test] Check for more -fsanitize=array-bounds behavior ...that had temporarily regressed with (since reverted)

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

sberg updated this revision to Diff 441640. sberg added a comment. added test involving template argument substitution CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128783/new/ https://reviews.llvm.org/D128783 Files: clang/test/CodeGen/bounds-checking-fam.c Index: clang/test/CodeGe

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

sberg marked an inline comment as done. sberg added inline comments. Comment at: clang/test/CodeGen/bounds-checking-fam.c:59 + // CXX-STRICT-0-NOT: @__ubsan + return p->a[i] + (p->a)[i]; +} serge-sans-paille wrote: > Another C++-specific behavior: if the bound

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sberg marked an inline comment as done. Closed by commit rG4996e3f68315: [test] Check for more -fsanitize=array-bounds behavior (authored by sberg). Repository: rG L

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

sberg added a comment. I'm surprised that $ cat test.c struct S { int m1; int m2[1]; }; void f(struct S * s) { s->m2[1] = 0; } $ clang -fsyntax-only -fstrict-flex-arrays=1 test.c test.c:6:5: warning: array index 1 is past the end of the array (which contains 1

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

sberg added a comment. With this commit, $ cat test.cc #include "boost/numeric/conversion/cast.hpp" int main() { return boost::numeric_cast(0L); } $ clang++ test.cc succeeds without any diagnostic, while with its parent commit https://github.com/llvm/llvm-project/commit/b364535304181

[PATCH] D119721: [clang][lex] Use `ConstSearchDirIterator` in lookup cache

sberg added subscribers: rnk, sberg. sberg added a comment. Herald added a project: All. I started to experience Clang crashing when building Firebird (as part of building LibreOffice) in clang-cl mode on Windows, and I think it is due to this change in combination with D2733

[PATCH] D35562: Support GNU extension StmtExpr in constexpr function

sberg updated this revision to Diff 111632. sberg added a comment. (the original diff had inadvertently lacked full -U99 context; uploaded a new diff) https://reviews.llvm.org/D35562 Files: clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX/constant-expression-cxx1y.cpp Index: clang/t

[PATCH] D35564: Suppress -pedantic warnings about GNU extension StmtExpr in glibc's assert macro

sberg updated this revision to Diff 111634. sberg added a comment. (the original diff had inadvertently lacked full -U99 context; uploaded a new diff) https://reviews.llvm.org/D35564 Files: clang/lib/Basic/DiagnosticIDs.cpp clang/test/SemaCXX/warn-sysheader-macro.cpp Index: clang/tes

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

sberg updated this revision to Diff 292728. sberg added a comment. ping (addressed the clang-tidy nitpick) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87030/new/ https://reviews.llvm.org/D87030 Files: clang/lib/AST/Expr.cpp clang/unittests/Tooling/CastExprTest.cpp Index: clang

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

sberg added a comment. ping^2 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87030/new/ https://reviews.llvm.org/D87030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

sberg created this revision. sberg added a reviewer: rsmith. Herald added a project: clang. sberg requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D87030 Files: clang/lib/AST/Expr.cpp clang/unittests/Tooling/CastExprTest.cpp Index: clang/uni

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

sberg added a comment. I hit this with a call to getSubExprAsWriten from the LibreOffice Clang plugin, have no idea whether it can be hit from within the compiler itself. Not sure if clang/unittests/Tooling/CastExprTest.cpp is the best place to add a test for it. Repository: rG LLVM Github

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

sberg added a comment. Ping; OK to submit? (I meanwhile also get this when compiling some code with plain Clang, not just with the LibreOffice Clang plugin where I saw it originally.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87030/new/ https://reviews.llvm.org/D87030 __

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

sberg added a comment. friendly ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87030/new/ https://reviews.llvm.org/D87030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89481: [scan-build] Fix clang++ pathname again

sberg created this revision. sberg added reviewers: aadg, dcoughlin. Herald added a subscriber: Charusso. Herald added a project: clang. sberg requested review of this revision. e00629f777d9d62875730f40d266727df300dbb2 "[scan-build] Fix clang++ pathname" had removed the -MAJOR.MINOR suffix, but si

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

sberg added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87030/new/ https://reviews.llvm.org/D87030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89481: [scan-build] Fix clang++ pathname again

sberg added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89481/new/ https://reviews.llvm.org/D89481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

[PATCH] D89481: [scan-build] Fix clang++ pathname again

sberg updated this revision to Diff 301273. sberg edited the summary of this revision. sberg added a comment. Is there a reason why "NoQ accepted this revision." kept this at "Needs Review" rather than moving it to "This revision is now accepted and ready to land."? Anyway, I added a test now.

[PATCH] D89481: [scan-build] Fix clang++ pathname again

This revision was automatically updated to reflect the committed changes. Closed by commit rG7a5184ed951a: [scan-build] Fix clang++ pathname again (authored by sberg). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89481/new/ https://reviews.llvm.org

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

sberg added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87030/new/ https://reviews.llvm.org/D87030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90572: [clang] [MinGW] Allow using the vptr sanitizer

sberg added a comment. Smells like this breaks various bots due to a -fsanitize=...,... option now listing 18 instead of 17 items, see http://lab.llvm.org:8011/#builders/76/builds/363, http://lab.llvm.org:8011/#builders/93/builds/430, http://lab.llvm.org:8011/#builders/66/builds/315, http://l

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

sberg added a comment. Is it intentional that this warns about volatile variables as in void f(char * p) { volatile char c = 0; c ^= *p; } (I see that GCC warns about volatile too, at least when you replace the `^=` with `=`, so assume the answer is "yes", but would just like to

[PATCH] D110310: State the kind of VerifyDiagnosticConsumer regex syntax (NFC)

sberg created this revision. sberg added a project: clang. sberg requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D110310 Files: clang/include/clang/Frontend/VerifyDiagnosticConsumer.h Index: clang/include/clang/Frontend/VerifyDiagnosticConsum

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

sberg created this revision. sberg added a reviewer: mbenfield. sberg added a project: clang. sberg requested review of this revision. At least LibreOffice has, for mainly historic reasons that would be hard to change now, a class `Any` with an overloaded `operator >>=` that semantically does no

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

sberg added a comment. In D103949#2807490 , @xbolva00 wrote: > gcc also ignores it? For reasons that I never looked into, GCC never warned for any of these cases. The Clang implementation appears to be way more aggressive (in a positive sense) to begi

[PATCH] D103623: [Clang] Test case for -Wunused-but-set-variable, warn for volatile.

sberg accepted this revision. sberg added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103623/new/ https://reviews.llvm.org/D103623 ___ c

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

sberg added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:7794-7798 +if (BO->getLHS()->getType()->isDependentType() || +BO->getRHS()->getType()->isDependentType()) +{ + if (BO->getOpcode() != BO_Assign) +return; mbenfiel

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb5b9489b2415: Only consider built-in compound assignment operators for -Wunused-but-set-* (authored by sberg). Changed prior to commit: https://re

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

sberg added a comment. (In a build prior to https://reviews.llvm.org/rGc60dd3b2626a4d9eefd9f82f9a406b0d28d3fd72 "Revert '[clang] NRVO: Improvements and handling of more cases.'") I see the following (reduced from https://git.libreoffice.org/core/+/649313625b94e6b879848fc19b607b74375100bf/o3tl/

  1   2   >