[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv created this revision. Herald added a subscriber: javed.absar. (Copy/pasting the reviewer list from https://reviews.llvm.org/D26856.) Addresses https://bugs.llvm.org/show_bug.cgi?id=30792 . In GCC, -mgeneral-regs-only emits errors upon trying to emit floating-point or vector op

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv added a comment. I like the idea of fixing those things, too; I'll start poking them soon. :) Even if we do end up fixing all of that, I still think it would be good to try to diagnose this in the frontend. So, if anyone has comments on this while I'm staring at the aarch64 ba

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv added a comment. > However, the tests cover floating point, but they don't cover vector calls > (arm_neon.h). `#include ` gives me ~12,000 errors, presumably because there are so many functions that take vectors/floats defined in it. The hope was that calling `bar` and `foo`

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv updated this revision to Diff 119499. george.burgess.iv marked 2 inline comments as done. george.burgess.iv added a comment. Addressed feedback. Thanks! After looking around and internalizing a little bit of how backends in LLVM work, the path forward I have in mind is to basic

[PATCH] D32332: Add support for transparent overloadable functions in clang

george.burgess.iv updated this revision to Diff 100201. george.burgess.iv added a comment. Herald added a subscriber: jfb. Fix the aforementioned issue; PTAL. Note that this fix is slightly backwards incompatible. It disallows code like: void foo(int); void foo(int) __attribute__((overloadab

[PATCH] D33904: Add a __has_attribute_enhancement macro to clang

george.burgess.iv created this revision. Herald added a subscriber: javed.absar. Attributes may gain features or added flexibility over time. This patch aims to add a simple and uniform way to directly add/query for arbitrary changes in attributes, instead of having to rely on other information

[PATCH] D32332: Add support for transparent overloadable functions in clang

george.burgess.iv added a comment. Ping :) https://reviews.llvm.org/D33904 is what I imagine would be a good way to detect this change. https://reviews.llvm.org/D32332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D33904: Add a __has_attribute_enhancement macro to clang

george.burgess.iv added a comment. > Why not just use __has_feature(overloadable_unmarked) or similar? My impression was that `__has_feature` was was for larger features than tweaks to attributes. If this would be an appropriate use of `__has_feature`, though, I'm happy to keep things simple.

[PATCH] D32332: Add support for transparent overloadable functions in clang

george.burgess.iv updated this revision to Diff 102207. george.burgess.iv added a comment. Swap to using `__has_feature(overloadable_unmarked)` for detection, as recommended by Hal in https://reviews.llvm.org/D33904 https://reviews.llvm.org/D32332 Files: include/clang/Basic/AttrDocs.td inc

[PATCH] D32332: Add support for transparent overloadable functions in clang

george.burgess.iv updated this revision to Diff 102850. george.burgess.iv marked 3 inline comments as done. george.burgess.iv added a comment. Addressed all feedback https://reviews.llvm.org/D32332 Files: include/clang/Basic/AttrDocs.td include/clang/Basic/DiagnosticSemaKinds.td include/c

[PATCH] D32332: Add support for transparent overloadable functions in clang

george.burgess.iv updated this revision to Diff 103448. george.burgess.iv marked 6 inline comments as done. george.burgess.iv added a comment. Addressed all feedback https://reviews.llvm.org/D32332 Files: include/clang/Basic/AttrDocs.td include/clang/Basic/DiagnosticSemaKinds.td lib/Lex/P

[PATCH] D32332: Add support for transparent overloadable functions in clang

george.burgess.iv added a comment. Thank you both for the comments! Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3294-3295 +def err_attribute_overloadable_multiple_unmarked_overloads : Error< + "at most one 'overloadable' function for a given name may lack the " + "

[PATCH] D41261: [libcxxabi][demangler] Special case demangling for pass_object_size attribute

george.burgess.iv added a comment. Thanks for working on this! Unfortunately, I've never touched demanglers before, so I don't think I can LGTM this patch. > This attribute is mangled as if it was meant to match the > production. Yup, definitely a mistake. Apologies. :) C

[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. LGTM assuming my nit gets addressed. Thank you! > Maybe someone who's more familiar with this builtin could point to the cause > of this discrepancy Yeah, the documenta

[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

george.burgess.iv added inline comments. Comment at: test/Sema/builtin-object-size.c:105 +void rd36094951_IAS_builtin_object_size_assertion(IncompleteArrayStruct* p) { + __builtin___strlcpy_chk (p->session[0].string, "ab", 2, __builtin_object_size(p->session[0].string, 1)); +}

[PATCH] D47840: Make -Wgcc-compat complain about declarations in for loop init statements

george.burgess.iv created this revision. george.burgess.iv added reviewers: rsmith, aaron.ballman. The following code is invalid before C99, since we try to declare `i` inside the first clause of the for loop: void foo() { for (int i = 0; i < 10; i++); } GCC does not accept this code in

[PATCH] D30760: Record command lines in objects built by clang, Clang part

george.burgess.iv closed this revision. george.burgess.iv added a comment. Herald added a subscriber: JDevlieghere. (Committed as noted by echristo; just trying to clean my queue a bit. :) ) https://reviews.llvm.org/D30760 ___ cfe-commits mailing li

[PATCH] D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.

george.burgess.iv accepted this revision. george.burgess.iv added a comment. Thanks! https://reviews.llvm.org/D52924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56760: Add a new builtin: __builtin_dynamic_object_size

george.burgess.iv added a comment. Thanks for this! > Would it make sense to model this as an (optional) extra flag bit for > __builtin_object_size rather than as a new builtin? +1. That way, we could avoid making a `pass_dynamic_object_size` (assuming we wouldn't want to have a different API

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

george.burgess.iv added a comment. Thanks for working on this! I hope to take an in-depth look at this patch next week (if someone else doesn't beat me to it...), but wanted to note that I think enabling clang to emit these warnings on its own is a good thing. `diagnose_if` is great for potent

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

george.burgess.iv added a comment. Looks solid to me overall; just a few nits. I don't think I have actual ownership over any of this code, so I'll defer to either Aaron or Richard for the final LGTM Thanks again! Comment at: clang/lib/Sema/SemaChecking.cpp:317 + // buffer

[PATCH] D59118: creduce script for clang crashes

george.burgess.iv added a comment. Thanks for this! Functionally, this looks good. My comments are mostly just simplicity/readability nitpicks. Comment at: clang/utils/creduce-clang-crash.py:36 + # Get clang call from build script + cmd = None + with open(build_script, 'r')

[PATCH] D59118: creduce script for clang crashes

george.burgess.iv added a comment. > I think we should do one more round of fixes, we can commit that for you, and > then move on to the next steps +1. This looks good to me with outstanding nits fixed > and filter out the # lines afterwards. AIUI, the very-recently-merged https://github.com/

[PATCH] D59118: creduce script for clang crashes

george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. I think that addresses all of the concerns people have put forward; given rnk's comment about one more round of fixes, this LGTM. Will check this in for you shortly. Tha

[PATCH] D59118: creduce script for clang crashes

This revision was automatically updated to reflect the committed changes. Closed by commit rC355944: Add a creduce script for clang crashes (authored by gbiv, committed by ). Changed prior to commit: https://reviews.llvm.org/D59118?vs=190285&id=190294#toc Repository: rC Clang CHANGES SINCE

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

george.burgess.iv added a comment. This LGTM; feel free to submit after Aaron stamps this. Thanks again! Comment at: clang/lib/Sema/SemaExpr.cpp:5929 +checkFortifiedBuiltinMemoryFunction(FDecl, TheCall); + erik.pilkington wrote: > george.burgess.iv wrote

[PATCH] D59440: add steps to preprocess file and reduce command line args

george.burgess.iv added a comment. Just a few style nits for you, and this LGTM. I assume rnk and serge-sans-paille are content, so I'm happy to check this in for you once these are addressed. Thanks! Comment at: clang/utils/creduce-clang-crash.py:64 crash_output, _ = p.c

[PATCH] D59440: add steps to preprocess file and reduce command line args

george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. LGTM; thanks again! Will land shortly CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59440/new/ https://reviews.llvm.org/D59440 __

[PATCH] D59440: add steps to preprocess file and reduce command line args

This revision was automatically updated to reflect the committed changes. Closed by commit rC356636: creduce-clang-crash.py: preprocess file + reduce commandline (authored by gbiv, committed by ). Changed prior to commit: https://reviews.llvm.org/D59440?vs=191598&id=191615#toc Repository: rC

[PATCH] D59725: Additions to creduce script

george.burgess.iv added a comment. Thanks for this! Comment at: clang/utils/creduce-clang-crash.py:137 + +# If no message was found, use the top five stack trace functions, +# ignoring some common functions Please expand a bit on why 5 was chosen (is th

[PATCH] D59725: Additions to creduce script

george.burgess.iv marked an inline comment as done. george.burgess.iv added inline comments. Comment at: clang/utils/creduce-clang-crash.py:223 + if len(x) > 0 and x[-1].startswith('-') and not y.startswith('-'): +x[-1] += ' ' + y +return x a

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

george.burgess.iv added a comment. We have warnings like -Wdivision-by-zero that take reachability into account: https://godbolt.org/z/3PD-eF. I don't immediately know how it's all done (e.g. in batch because CFG construction is expensive? ...), but looking there for inspiration may be useful.

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

george.burgess.iv added a comment. (Forgot to refresh before pressing 'Submit', so maybe efriedma's comment answers all of the questions in mine ;) ) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58797/new/ https://reviews.llvm.org/D58797

[PATCH] D59725: Additions to creduce script

george.burgess.iv added a comment. Only a few more nits on my side, and this LGTM. WDYT, arichardson? Comment at: clang/utils/creduce-clang-crash.py:137 + +# If no message was found, use the top five stack trace functions, +# ignoring some common functions -

[PATCH] D59725: Additions to creduce script

This revision was automatically updated to reflect the committed changes. Closed by commit rL357290: Various fixes and additions to creduce-clang-crash.py (authored by gbiv, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://r

[PATCH] D61750: [Targets] Move soft-float-abi filtering to `initFeatureMap`

george.burgess.iv created this revision. george.burgess.iv added reviewers: michaelplatings, efriedma. Herald added subscribers: kristof.beyls, javed.absar. Herald added a project: clang. I'm not convinced this is an excellent approach, in part because I'm unclear on where all we expect to funnel

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

george.burgess.iv added a comment. Thanks for this! LGTM after erichkeane's comments are resolved. > I did a little digging on this, and it seems to be to keep track of a > declarations linkage for caching sake Yeah, otherwise, we get exponential behavior on some pathological template-y patter

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

george.burgess.iv accepted this revision. george.burgess.iv added a comment. LGTM too. Thanks again! Repository: rC Clang https://reviews.llvm.org/D52268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

george.burgess.iv added a comment. Will submit once gribozavr indicates that they're happy with the new test names. Thanks again for working on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/ https://reviews.llvm.org/D61967 _

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

This revision was automatically updated to reflect the committed changes. Closed by commit rL362673: android: add a close-on-exec check on pipe() (authored by gbiv, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

This revision was automatically updated to reflect the committed changes. Closed by commit rL362672: android: add a close-on-exec check on pipe2() (authored by gbiv, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.l

[PATCH] D61750: [Targets] Move soft-float-abi filtering to `initFeatureMap`

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

[PATCH] D61750: [Targets] Move soft-float-abi filtering to `initFeatureMap`

This revision was automatically updated to reflect the committed changes. Closed by commit rL363346: [Targets] Move soft-float-abi filtering to `initFeatureMap` (authored by gbiv, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: htt

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

george.burgess.iv added a comment. Thanks for this! Mostly just nitpicking the warning's wording. :) Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776 +def warn_alloca : Warning< + "use of builtin function %0">, + InGroup>, DefaultIgnore; nit

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

george.burgess.iv added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776 +def warn_alloca : Warning< + "use of builtin function %0">, + InGroup>, DefaultIgnore; aaron.ballman wrote: > george.burgess.iv wrote: > > nit: I'd just

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

george.burgess.iv added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776 +def warn_alloca : Warning< + "use of builtin function %0">, + InGroup>, DefaultIgnore; aaron.ballman wrote: > george.burgess.iv wrote: > > aaron.ballman

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

george.burgess.iv added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776 +def warn_alloca : Warning< + "use of builtin function %0">, + InGroup>, DefaultIgnore; aaron.ballman wrote: > george.burgess.iv wrote: > > aaron.ballman

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

This revision was automatically updated to reflect the committed changes. Closed by commit rL367067: [Sema] add -Walloca to flag uses of `alloca` (authored by gbiv, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D63623: [clang-tidy] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

george.burgess.iv added a comment. just a few drive-by nits/comments from me. as usual, not super familiar with clang-tidy, so i won't be able to stamp this. thanks! Comment at: clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp:23 + binaryOperator( + has

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

george.burgess.iv added a comment. Thanks for this! I don't have great context on tidy, so I can't stamp this, but I do have a few drive-by nits for you. Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:22 + functionDecl(returns(isIn

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

george.burgess.iv added inline comments. Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:27 +void CloexecPipeCheck::check(const MatchFinder::MatchResult &Result) { + const std::string &ReplacementText = + (Twine("pipe2(") + getSpellingArg(Result, 0) +

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv added a comment. I'm happy to give rebasing it a shot later this week. My recollection of the prior state of this patch was that we wanted some backend work done to double-check that no illegal ops get generated by optimizations and such, since these checks are purely done in

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv updated this revision to Diff 200388. george.burgess.iv added a comment. Rebased CHANGES SINCE LAST ACTION https://reviews.llvm.org/D38479/new/ https://reviews.llvm.org/D38479 Files: clang/docs/UsersManual.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/inc

[PATCH] D61750: [Targets] Move soft-float-abi filtering to `initFeatureMap`

george.burgess.iv updated this revision to Diff 200862. george.burgess.iv added a comment. Address comments -- thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61750/new/ https://reviews.llvm.org/D61750 Files: clang/lib/Basic/Targets/ARM.cpp clang/lib/Basic/Targets/ARM.h cla

[PATCH] D61750: [Targets] Move soft-float-abi filtering to `initFeatureMap`

george.burgess.iv added a comment. > neonfp isn't passed as a feature in the first place; there's a separate API > setFPMath which is used for that. We translate it into a target feature for > the sake of the backend. So I'm not sure what you're proposing. The idea would be for `initFeatureMap`

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv updated this revision to Diff 201047. george.burgess.iv marked 10 inline comments as done. george.burgess.iv added a comment. Addressed feedback, modulo the constant foldable comment thread. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D38479/new/ https://reviews.llvm.

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv added a comment. Thanks for the feedback! > With this patch, do we pass the general-regs-only attribute to the backend? > If so, would that be the attribute we'd want to check to emit errors from the > backend from any "accidental" floating-point operations? Yeah, the current

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv added a comment. Apologies for the latency of my updates. > Something I ran into when reviewing https://reviews.llvm.org/D62639 is that > on AArch64, for varargs functions, we emit floating-point stores when > noimplicitfloat is specified. That seems fine for -mno-implicit-flo

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv updated this revision to Diff 220416. george.burgess.iv marked 4 inline comments as done. george.burgess.iv added a reviewer: efriedma. george.burgess.iv added a comment. Chatted with Eli offline; updated here to reflect the conclusions of that. Importantly, this patch readds so

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv updated this revision to Diff 222525. george.burgess.iv marked 6 inline comments as done. george.burgess.iv added a comment. Herald added subscribers: llvm-commits, hiraditya. Herald added a project: LLVM. Addressed feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

george.burgess.iv added a comment. > Does this have any significant impact on -fsyntax-only performance? I'm sure there are pathological cases where this hurts perf, but my intuition tells me that we won't get bitten badly by any of them in the real world. It should be a branch per cast + full

[PATCH] D47840: Make -Wgcc-compat complain about declarations in for loop init statements

george.burgess.iv added a comment. Ping :) Repository: rC Clang https://reviews.llvm.org/D47840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47840: Make -Wgcc-compat complain about declarations in for loop init statements

george.burgess.iv added a comment. Thanks! Repository: rC Clang https://reviews.llvm.org/D47840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47840: Make -Wgcc-compat complain about declarations in for loop init statements

This revision was automatically updated to reflect the committed changes. Closed by commit rL335927: [Parse] Make -Wgcc-compat complain about for loop inits in C89 (authored by gbiv, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D4784

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

george.burgess.iv added a comment. For a more direct comparison, I offer https://godbolt.org/z/fqAhUC . The lack of optimization in the later case is because we're forced to mark the call to `__builtin_memcpy` in the inline memcpy as `nobuiltin`. If we instead rename things, this issue doesn't

[PATCH] D78148: [CodeGen] make isTriviallyRecursive handle more trivial recursion

george.burgess.iv created this revision. george.burgess.iv added reviewers: efriedma, serge-sans-paille. george.burgess.iv added a project: clang. Herald added a subscriber: cfe-commits. This function was not catching all forms of trivial recursion, meaning: void *memcpy(void *a, const void *b,

[PATCH] D78148: [CodeGen] make isTriviallyRecursive handle more trivial recursion

george.burgess.iv abandoned this revision. george.burgess.iv added a comment. Nothing in the real world :) I was writing test-cases for a soon-to-be-in-your-inbox patch, and initially wrote `memcpy` recursion in terms of `memcpy`, rather than `__builtin_memcpy`. Wasn't sure if this was an overs

[PATCH] D78162: [CodeGen] Mark inline definitions of builtins as nobuiltin only if we plan to emit them.

george.burgess.iv created this revision. george.burgess.iv added reviewers: efriedma, serge-sans-paille. george.burgess.iv added a project: clang. Herald added a subscriber: cfe-commits. This suboptimality was encountered in Linux (see some discussion on D71082 ,

[PATCH] D78162: [CodeGen] Mark inline definitions of builtins as nobuiltin only if we plan to emit them.

george.burgess.iv added a comment. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78162/new/ https://reviews.llvm.org/D78162 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D78162: [CodeGen] Mark inline definitions of builtins as nobuiltin only if we plan to emit them.

This revision was automatically updated to reflect the committed changes. Closed by commit rG2dd17ff08165: [CodeGen] only add nobuiltin to inline builtins if we'll emit them (authored by george.burgess.iv). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry.

george.burgess.iv added a reviewer: alexfh. george.burgess.iv added a comment. Concept and implementation LGTM. Please wait for comment from +alexfh before landing, since I think they have more ownership over clang-tidy in general than I do :) Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D78162: [CodeGen] Mark inline definitions of builtins as nobuiltin only if we plan to emit them.

george.burgess.iv added a comment. Thanks for the report! Looking now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78162/new/ https://reviews.llvm.org/D78162 ___ cfe-commits mailing list cfe-commits@

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

george.burgess.iv added a comment. Thanks for this! It's interesting to me that these array-bound checks don't seem to use `@llvm.objectsize` in some form already. I can't find any notes about it grepping through git/source, so I'm happy with it. Comment at: lib/CodeGen/CGEx

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. LGTM. Thanks again! https://reviews.llvm.org/D40940 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

[PATCH] D40941: [ubsan] Use pass_object_size info in bounds checks (compiler-rt)

george.burgess.iv added a comment. If the answer to my question is "no, it'll just work," LGTM. Thanks! Comment at: test/ubsan/TestCases/Misc/bounds.cpp:9 +int get_int(int *const p __attribute__((pass_object_size(0))), int i) { + // CHECK-A-2: bounds.cpp:[[@LINE+1]]:10: runtim

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

george.burgess.iv added a comment. > That said, one of the upsides of the current ubsan is that whether it will > produce a diagnostic is predictable (as long as you don't use uninitialized > data); you lose that to some extent with llvm.objectsize because it depends > on the optimizer. True.

[PATCH] D106030: [Clang] add support for error+warning fn attrs

george.burgess.iv added a comment. > Adding something to the IR for the sole purpose of producing a diagnostic > feels really weird. I'm not sure I see why the frontend can't see this > attribute and directly warn To add a bit more clarification, the goal of this attribute is specifically to e

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. lgtm -- thanks! please give a day for other reviewers to add any last minute comments, then i think we can land this. Comment at: clang/lib/Sema/SemaC

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe12e02df09a9: [clang] Evaluate strlen of strcpy argument for -Wfortify-source. (authored by mbenfield, committed by gbiv). Repository: rG LLVM Git

[PATCH] D126735: [clang-tidy] Silence modernize-redundant-void-arg in the case of vexing parses

george.burgess.iv updated this revision to Diff 444397. george.burgess.iv added a comment. Rebased on top of 891319f654c102572cf7028ed04bbaf6c59e7bff as requested; `ninja check-clang-extra docs-clang-tools-html` passes CHANG

[PATCH] D91372: Some updates/fixes to the creduce script.

george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang/utils/creduce-clang-crash.py:156 + def skip_function(func_name): +for name in filters: + if name in func

[PATCH] D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry.

This revision was automatically updated to reflect the committed changes. Closed by commit rG9d40fb808fd0: Allow to specify macro names for android-comparison-in-temp-failure-retry (authored by fmayer, committed by george.burgess.iv). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D89988: adds basic -Wfree-nonheap-object functionality

This revision was automatically updated to reflect the committed changes. Closed by commit rG425a83a5f069: [Sema] adds basic -Wfree-nonheap-object functionality (authored by cjdb, committed by george.burgess.iv). Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D90269: adds -Wfree-nonheap-object member var checks

This revision was automatically updated to reflect the committed changes. Closed by commit rGba18bc4925d8: [Sema] adds -Wfree-nonheap-object member var checks (authored by cjdb, committed by george.burgess.iv). Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGcf49cae278b4: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable (authored by mbenfield, committed by george.burgess.iv). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG20f7b5f3f9c8: [Clang] Test case for -Wunused-but-set-variable, warn for volatile. (authored by mbenfield, committed by george.burgess.iv). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D99993: [clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers

george.burgess.iv added a comment. ...This entirely dropped off my radar. Will try to land it now; thanks, all! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D3/new/ https://reviews.llvm.org/D3 __

[PATCH] D99993: [clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers

This revision was automatically updated to reflect the committed changes. Closed by commit rG2e75986a21e5: bugprone-argument-comment: ignore mismatches from system headers (authored by gbiv). Changed prior to commit: https://reviews.llvm.org/D3?vs=335660&id=363846#toc Repository: rG LLVM

[PATCH] D104887: [clang] Evaluate strlen of strcpy argument for -Wfortify-source.

george.burgess.iv added a comment. thanks for this! mostly just nits from me Comment at: clang/lib/AST/ExprConstant.cpp:15755 + +bool Expr::tryEvaluateStrLen(uint64_t &Result, ASTContext &Ctx) const { + Expr::EvalStatus Status; Looks like this is the second "t

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. LGTM % 2 nits. Please feel free to commit after it LGT @aaron.ballman too. :) Thanks again! Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.t

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

george.burgess.iv added inline comments. Comment at: clang/test/Sema/attr-diagnose-as-builtin.c:62 + +#ifdef __cplusplus +template mbenfield wrote: > george.burgess.iv wrote: > > nit: can we also add a non-templated overload check in here? > > > > if the diag i

[PATCH] D126735: [clang-tidy] Silence modernize-redundant-void-arg in the case of vexing parses

george.burgess.iv created this revision. george.burgess.iv added reviewers: LegalizeAdulthood, aaron.ballman. george.burgess.iv added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. george.burgess.iv requested review of this revision. H

[PATCH] D75492: [modernize-use-using] Don't diagnose typedefs in `extern "C"` DeclContexts

george.burgess.iv created this revision. george.burgess.iv added a reviewer: aaron.ballman. If code is shared between C and C++, converting a `typedef` to a `using` isn't possible. Being more conservative about emitting these lints in `extern "C"` blocks seems like a good compromise to me. htt

[PATCH] D75492: [clang-tidy]: modernize-use-using: don't diagnose typedefs in `extern "C"` DeclContexts

george.burgess.iv planned changes to this revision. george.burgess.iv added a comment. I see -- that seems like a much broader change, but also probably worthwhile. I have a few ideas about how to tackle that; let me see what I can come up with locally. Thanks! Repository: rG LLVM Github Mo

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

george.burgess.iv added a comment. Thanks for looking into this! I dunno what implications this has elsewhere, but I'd like to note a few things about this general approach: - AIUI, this'll only work for FORTIFY functions which are literally calling `__builtin___foo_chk`; an equivalent impleme

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

george.burgess.iv added a comment. Should we also have a quick test-case in Sema/ verifying that we still get the warnings that Eli mentioned? Comment at: clang/lib/AST/Decl.cpp:3006 +bool FunctionDecl::isReplaceableSystemFunction() const { + FunctionDecl const *Definition;

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

george.burgess.iv added inline comments. Comment at: clang/lib/AST/Decl.cpp:3006 +bool FunctionDecl::isReplaceableSystemFunction() const { + FunctionDecl const *Definition; serge-sans-paille wrote: > george.burgess.iv wrote: > > serge-sans-paille wrote: > > >

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

george.burgess.iv added a comment. Just a few more nits, and this LGTM. Thanks again! IDK if Eli wanted to have another look at this; please wait until tomorrow before landing to give him time to comment. Comment at: clang/lib/AST/Decl.cpp:3006 +bool FunctionDecl::isInlineB

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

george.burgess.iv added a comment. Until recently, both CrOS and Android disabled FORTIFY wholesale when any of asan/msan/tsan were active. Bionic recently got support for FORTIFY playing nicely with sanitizers, but that support boils down to "turn off all the FORTIFY runtime checks, but leave

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

george.burgess.iv added a comment. (It's interesting to me if gcc doesn't warn about that libcxx code, since the whole point of the gnuc 5.0 check there was "the compiler should check this for us now"...) If a glibc-side fix for this does materialize, IMO `-fgnuc-version=5.0` isn't a good path

  1   2   >