Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

jfb (JF Bastien)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 27 2014, 9:36 AM (503 w, 4 d)

Recent Activity

Jul 18 2023

jfb added a comment to D155580: [trivial-auto-var-init] Do not emit initialization code for empty class.

(sorry for sending twice, looks like email reply failed)

Jul 18 2023, 2:08 PM · Restricted Project, Restricted Project

Nov 22 2022

jfb added a comment to D137707: Move "auto-init" instructions to the dominator of their users.

Are there any worries about moving stores that are on atomic/volatile "objects", either because the auto-init store itself is atomic/volatile, or because a subsequent access is marked as such? I don't think there's a worry because auto-init stores are technically outside the abstract machine (they "don't exist"), but I'm not 100% convinced.

Nov 22 2022, 4:16 PM · Restricted Project, Restricted Project

May 11 2022

jfb updated subscribers of D125142: [clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang.

I think the most relevant post from @rsmith is: https://discourse.llvm.org/t/making-ftrivial-auto-var-init-zero-a-first-class-option/55143/40

May 11 2022, 4:18 AM · Restricted Project, Restricted Project

Dec 13 2021

jfb added a comment to D115440: Provide __builtin_alloca*_uninitialized variants.

GCC devs say that initializing explicit alloca() is a bug, because they aren't "automatic storage": https://lkml.kernel.org/r/20211209201616.GU614@gate.crashing.org
.. which is also the reason why GCC's behaviour differs here at the moment.

I actually agree with that, and alloca auto-init behaviour needs a new -mllvm param.

Dec 13 2021, 5:47 PM · Restricted Project

Sep 27 2021

jfb added a comment to D108469: Improve handling of static assert messages..

Can you test all the values in this? https://godbolt.org/z/h7n54fa5x

Sep 27 2021, 9:02 AM · Restricted Project, Restricted Project, Restricted Project

Sep 8 2021

jfb added inline comments to D75960: [libc++] Implement C++20's P0476r2: std::bit_cast.
Sep 8 2021, 4:54 PM · Restricted Project

Aug 20 2021

jfb updated subscribers of D108469: Improve handling of static assert messages..

I worry that changing the general static_assert printing (adding a colon, and dropping the quotes) will get @hwright's law to drop on us. We can try and see if e.g. users of clang have automated checks for static_assert in their CI pipelines or something. I think your new format looks better, but Hyrum is finicky that way... What do others think?

Aug 20 2021, 9:45 AM · Restricted Project, Restricted Project, Restricted Project

Jul 12 2021

jfb updated subscribers of D105338: [InstCombine] Revert "Temporarily do not drop volatile stores before unreachable".

I agree with @eli.friedman's breakdown of options here: https://reviews.llvm.org/D105338#2870156

Jul 12 2021, 9:02 AM · Restricted Project

Jun 26 2021

jfb added a comment to D104975: Implement P1949.

It would be more user friendly to say which character is not allowed in the diagnostic.

Jun 26 2021, 6:42 AM · Restricted Project, Restricted Project, Restricted Project

Jun 15 2021

jfb added a comment to D89353: Enable overriding `__libcpp_debug_function` invocation.

FWIW I support something along the lines of what Palmer is trying to do: many users of the STL would like to be able to handle particular types of library UB instead of performing potentially unbounded effects. Which ones is a bit of an art that we need to figure out, for example I wouldn't want to unduly affect performance (by e.g. changing algorithmic complexity) or code size (here, with diagnostic bloat). In a way we're experimenting on the edges of what contract checking would be for the STL, and I would approach it with this optic: could we have a mode of the STL which enforces contracts, and a mode where you get whatever. If so, what should be checked and what should remain unchecked, and what are the principles to distinguish these. I think in this discussion it's important to separate the invariants that are details of libc++'s implementation, from what are actual contracts at the STL API level.

Jun 15 2021, 3:06 PM · Restricted Project, Restricted Project

May 4 2021

jfb added a comment to D101501: [CGAtomic] Lyft strong requirement for remaining compare_exchange combinations.

Don't mind JF, he's just forgotten that C++ lacks the ability to turn the typically constant value arguments passed to functions like std::atomic::compare_exchange_strong into actual compile-time constants to the builtin used within those functions, so his advice would mean emitting all atomics in C++ as sequentially consistent.

May 4 2021, 2:01 PM · Restricted Project

May 3 2021

jfb added inline comments to D101501: [CGAtomic] Lyft strong requirement for remaining compare_exchange combinations.
May 3 2021, 5:46 PM · Restricted Project
jfb accepted D101501: [CGAtomic] Lyft strong requirement for remaining compare_exchange combinations.
May 3 2021, 4:39 PM · Restricted Project
jfb updated subscribers of D96790: [libc++] Enable the synchronization library on Apple platforms.

I would recommend checking with @MadCoder about the implementation and ABI, since the kernel has Opinions on this. IIRC he looked at Olivier's original patch.

May 3 2021, 1:34 PM · Restricted Project

Apr 16 2021

jfb added a reviewer for D100465: clang: Remove __atomic libstdc++ hack: jwakely.
Apr 16 2021, 9:25 AM · Restricted Project

Apr 7 2021

jfb accepted D100057: Remove warning "suggest braces" for aggregate initialization of an empty class with an aggregate base class..

A few nits, but this is good otherwise!

Apr 7 2021, 1:45 PM · Restricted Project

Mar 24 2021

jfb accepted D99232: [Nomination] Adding new Google representative to security group.

I'm no longer in the security group, but (from the peanut gallery) I endorse Google having a second representative, and George in particular.

Mar 24 2021, 9:36 AM · Restricted Project

Mar 20 2021

jfb accepted D98995: [CGAtomic] Lift stronger requirements on cmpxch and add support for acquire failure mode.

Maybe refer to http://wg21.link/p0418 directly in the commit message?

Mar 20 2021, 1:39 PM · Restricted Project, Restricted Project
jfb added a reviewer for D98995: [CGAtomic] Lift stronger requirements on cmpxch and add support for acquire failure mode: kubamracek.
Mar 20 2021, 1:38 PM · Restricted Project, Restricted Project

Feb 19 2021

jfb added a comment to D97044: [libc++] [C++2b] [P0943] Add stdatomic.h header..

Does it line up property with ./clang/lib/Headers/stdatomic.h ? I think this other header might need some adjustment as well, for example the hosted/freestanding part.

Feb 19 2021, 10:04 AM · Restricted Project, Restricted Project

Dec 2 2020

jfb accepted D92515: Bump MSVC required version to 19.14.

LGTM. Maybe someone like @STL_MSFT can see if MS folks are OK with this change.

Dec 2 2020, 5:24 PM · Restricted Project
jfb added a comment to D92500: ADT: Replace guts of AlignedCharArrayUnion with std::aligned_union_t, NFC.

@jfb, any reason you didn't gut this in 80b67baaedd50ded121908f9a4c03f5939b84f24?

Dec 2 2020, 1:57 PM · Restricted Project

Nov 23 2020

jfb added a comment to D91992: [libc++] Add an extension to allow constructing std::thread from native handles.

This is a sensible thing to support IMO. It's indeed weird to have native handle in other places, but not here.

Nov 23 2020, 2:24 PM · Restricted Project
jfb updated subscribers of D91992: [libc++] Add an extension to allow constructing std::thread from native handles.

This is a sensible thing to support IMO. It's indeed weird to have native handle in other places, but not here.

Nov 23 2020, 2:06 PM · Restricted Project

Nov 20 2020

jfb added a comment to D87974: [Builtin] Add __builtin_zero_non_value_bits..

Let's make sure that we follow the same semantics that GCC does, particularly w.r.t. union, bitfields, and padding at the end of an object (whether it's in an array or not).

Nov 20 2020, 1:49 PM · Restricted Project, Restricted Project

Nov 19 2020

jfb added a comment to D75229: [clang-tidy] Add signal-in-multithreaded-program check.

Most of the tests as written should be failing right now, at least on macOS and Linux, because they likely should be identified as POSIX, right?

Nov 19 2020, 3:00 PM · Restricted Project, Restricted Project

Nov 10 2020

jfb added inline comments to D91156: [AArch64] Compiler-rt interface for out-of-line atomics..
Nov 10 2020, 8:48 AM · Restricted Project

Oct 8 2020

jfb added a comment to D79279: Add overloaded versions of builtin mem* functions.

ping

Oct 8 2020, 1:57 PM · Restricted Project, Restricted Project

Sep 23 2020

jfb added inline comments to D87974: [Builtin] Add __builtin_zero_non_value_bits..
Sep 23 2020, 10:10 AM · Restricted Project, Restricted Project

Sep 19 2020

jfb added inline comments to D87974: [Builtin] Add __builtin_zero_non_value_bits..
Sep 19 2020, 4:14 PM · Restricted Project, Restricted Project

Sep 18 2020

jfb added a comment to D87858: [hip] Add HIP scope atomic ops..
In D87858#2280429, @jfb wrote:

Please provide documentation in this patch.

opencl atomic builtins are documented as notes to __c11_atomic builtins part of https://clang.llvm.org/docs/LanguageExtensions.html#builtin-functions. These new atomic builtins can be documented in a similar way after that.

Sep 18 2020, 9:52 AM · Restricted Project

Sep 17 2020

jfb added a comment to D87858: [hip] Add HIP scope atomic ops..

Please provide documentation in this patch.

Sep 17 2020, 3:04 PM · Restricted Project

Sep 8 2020

jfb updated subscribers of D87323: Bring atomic header closer to C++20.

Generally this looks good, but I didn't go into details.
It would be good to have an MS person like @STL_MSFT check that the std tests match, at least for what they've implemented.

Sep 8 2020, 12:55 PM · Restricted Project
jfb added a comment to D78938: Make LLVM build in C++20 mode.

On C++20 mode rotting: it won't if someone sets up a bot. If it rots, then it's easier to un-rot with Barry's patch.

Sep 8 2020, 10:26 AM · Restricted Project, Restricted Project

Sep 7 2020

jfb added a comment to D79279: Add overloaded versions of builtin mem* functions.

ping

Sep 7 2020, 11:04 AM · Restricted Project, Restricted Project

Sep 3 2020

jfb committed rGbaa74e013f7e: Step down from security group (authored by jfb).
Step down from security group
Sep 3 2020, 8:50 AM
jfb closed D86742: Step down from security group.
Sep 3 2020, 8:50 AM · Restricted Project

Sep 1 2020

jfb added a comment to D86917: [Asan] Cleanup atomic usage in allocator.
In D86917#2249793, @jfb wrote:

Is this really NFC?

I guess so, do you have particular concern?
I don't know particular cases where it was broken, but reading values some times as atomic and sometimes as not, plus unclear order of bit fields looks suspicions.

Sep 1 2020, 2:06 PM · Restricted Project
jfb added a comment to D86917: [Asan] Cleanup atomic usage in allocator.

Is this really NFC?

Sep 1 2020, 9:06 AM · Restricted Project

Aug 29 2020

jfb added a reviewer for D79279: Add overloaded versions of builtin mem* functions: rsmith.
Aug 29 2020, 3:52 PM · Restricted Project, Restricted Project
jfb requested changes to D33825: [clang-tidy] signal handler must be plain old function check.

MSC54-CPP refers to POF, which as I pointed out above isn't relevant anymore. I'd much rather have a diagnostic which honors the state of things after http://wg21.link/p0270.

Aug 29 2020, 3:47 PM · Restricted Project

Aug 28 2020

jfb requested changes to D33825: [clang-tidy] signal handler must be plain old function check.

Please consider these changes, and whether this is relevant as implemented: http://wg21.link/p0270

Aug 28 2020, 8:46 AM · Restricted Project

Aug 27 2020

jfb updated the summary of D86742: Step down from security group.
Aug 27 2020, 8:16 PM · Restricted Project
jfb committed rG82d29b397bb2: Add an unsigned shift base sanitizer (authored by jfb).
Add an unsigned shift base sanitizer
Aug 27 2020, 8:07 PM
jfb closed D86000: Add an unsigned shift base sanitizer.
Aug 27 2020, 8:06 PM · Restricted Project, Restricted Project, Restricted Project
jfb added a comment to D86742: Step down from security group.

I'll wait 5 days to do this, since our process says:

Aug 27 2020, 3:54 PM · Restricted Project
jfb updated subscribers of D86742: Step down from security group.
Aug 27 2020, 3:05 PM · Restricted Project
jfb requested review of D86742: Step down from security group.
Aug 27 2020, 3:02 PM · Restricted Project
jfb added inline comments to D86000: Add an unsigned shift base sanitizer.
Aug 27 2020, 1:32 PM · Restricted Project, Restricted Project, Restricted Project
jfb updated the diff for D86000: Add an unsigned shift base sanitizer.

Remove the "suppress this" in release notes.

Aug 27 2020, 1:32 PM · Restricted Project, Restricted Project, Restricted Project
jfb added inline comments to D86000: Add an unsigned shift base sanitizer.
Aug 27 2020, 10:38 AM · Restricted Project, Restricted Project, Restricted Project
jfb updated the diff for D86000: Add an unsigned shift base sanitizer.

Fix notes.

Aug 27 2020, 10:37 AM · Restricted Project, Restricted Project, Restricted Project
jfb added inline comments to D86000: Add an unsigned shift base sanitizer.
Aug 27 2020, 10:20 AM · Restricted Project, Restricted Project, Restricted Project
jfb updated the diff for D86000: Add an unsigned shift base sanitizer.

Address comments

Aug 27 2020, 10:19 AM · Restricted Project, Restricted Project, Restricted Project

Aug 26 2020

jfb updated the summary of D86000: Add an unsigned shift base sanitizer.
Aug 26 2020, 4:41 PM · Restricted Project, Restricted Project, Restricted Project
jfb updated the diff for D86000: Add an unsigned shift base sanitizer.

Re-upload with full context.

Aug 26 2020, 4:41 PM · Restricted Project, Restricted Project, Restricted Project
jfb updated the diff for D86000: Add an unsigned shift base sanitizer.

As Vedant suggested, make it part of 'integer' sanitizer.

Aug 26 2020, 4:39 PM · Restricted Project, Restricted Project, Restricted Project
jfb added a comment to D79279: Add overloaded versions of builtin mem* functions.

Thanks, I'm happy with this approach.

If I understand correctly, the primary (perhaps sole) purpose of __builtin_memcpy_sized is to provide a primitive from which an atomic operation can be produced. That being the case, I wonder if the name is emphasizing the wrong thing, and a name that contains atomic would be more obvious. As a concrete alternative, __atomic_unordered_memcpy is not much longer and seems to have the right kinds of implications. WDYT?

Aug 26 2020, 4:25 PM · Restricted Project, Restricted Project
jfb updated the diff for D79279: Add overloaded versions of builtin mem* functions.

Address Richard's comments.

Aug 26 2020, 4:22 PM · Restricted Project, Restricted Project
jfb abandoned D23041: Un-XFAIL GCC atomics.align.
Aug 26 2020, 8:41 AM
jfb abandoned D50593: ConstantMerge: merge common initial sequences.

This is probably still useful, if someone wants to pick it up.

Aug 26 2020, 8:41 AM · Restricted Project
jfb abandoned D65846: Improve error message from FrontendAction.
Aug 26 2020, 8:40 AM · Restricted Project
jfb abandoned D65493: Modernize atomic detection and usage.
Aug 26 2020, 8:39 AM · Restricted Project, Restricted Project, Restricted Project
jfb abandoned D66244: Compiler.h: remove old GCC checks, update docs.
Aug 26 2020, 8:39 AM · Restricted Project
jfb closed D65545: Handle some fs::remove failures.
Aug 26 2020, 8:38 AM · Restricted Project, Restricted Project
jfb abandoned D77219: UBSan ␇ runtime.

I don't think the world is ready for this.

Aug 26 2020, 8:38 AM · Restricted Project, Restricted Project

Aug 24 2020

jfb added a comment to D79279: Add overloaded versions of builtin mem* functions.

Ping, I think I've addressed all comments here.

Aug 24 2020, 10:59 AM · Restricted Project, Restricted Project

Aug 18 2020

jfb accepted D86043: [compiler-rt][builtins] Tweak checks for lock-free atomics.
In D86043#2222622, @jfb wrote:

Can you add a test that this works, e.g. by having a program which uses these functions, but doesn't link to libatomic?

This can be tested by my previous (under review) D81348. With this patch you can compile and link that test from D81348 when targeting RISC-V rv32imac, while without this patch that test will fail to link. (Tested with a complementary GNU baremetal newlib toolchain, which doesn't even have libatomic to link to.)

Aug 18 2020, 8:31 AM · Restricted Project

Aug 17 2020

jfb added a comment to D86043: [compiler-rt][builtins] Tweak checks for lock-free atomics.

Can you add a test that this works, e.g. by having a program which uses these functions, but doesn't link to libatomic?

Aug 17 2020, 2:41 PM · Restricted Project

Aug 14 2020

jfb added a comment to D86000: Add an unsigned shift base sanitizer.
In D86000#2219288, @vsk wrote:

It'd be nice to fold the new check into an existing sanitizer group to bring this to a wider audience. Do you foresee adoption issues for existing -fsanitize=integer adopters? Fwiw some recently-added implicit conversion checks were folded in without much/any pushback.

Aug 14 2020, 4:05 PM · Restricted Project, Restricted Project, Restricted Project
jfb updated subscribers of D86000: Add an unsigned shift base sanitizer.
Aug 14 2020, 2:53 PM · Restricted Project, Restricted Project, Restricted Project
jfb requested review of D86000: Add an unsigned shift base sanitizer.
Aug 14 2020, 2:51 PM · Restricted Project, Restricted Project, Restricted Project
jfb accepted D84049: Disable use of _ExtInt with '__atomic' builtins.

@ldionne should sign off on the libcxx test, but the rest seems fine to me. Eventually we probably want to enable something, but need to be consistent about it.

Aug 14 2020, 1:28 PM · Restricted Project, Restricted Project
jfb accepted D85975: [NFC] Fix typo and variable names.

The typo fix is fine, I'm not sure it's worth fixing variable names (since there's an ongoing discussion about renaming everything anyways).
FWIW C++ doesn't really have a lattice anymore: https://wg21.link/p0418

Aug 14 2020, 10:34 AM · Restricted Project

Aug 13 2020

jfb updated subscribers of D85900: [mlir] do not use llvm.cmpxchg with floats.

See for example r360421 D58251 D50491 D26266 D26266 r264845 D15471
Maybe talk to folks such as @reames ?

Aug 13 2020, 3:00 PM · Restricted Project
jfb added a comment to D79279: Add overloaded versions of builtin mem* functions.

Actually I think any subsequent updates to tests can be done in a follow-up patch, since I'm not changing the status-quo on address space here.

Aug 13 2020, 2:09 PM · Restricted Project, Restricted Project
jfb updated the diff for D79279: Add overloaded versions of builtin mem* functions.

Fix a test.

Aug 13 2020, 1:11 PM · Restricted Project, Restricted Project
jfb added a comment to D85900: [mlir] do not use llvm.cmpxchg with floats.

I'd expect us to go the other way, and allow this. There have been patches in the past to allow usage of atomics with float. Could you look with their authors and see what they advise here?

Aug 13 2020, 11:31 AM · Restricted Project
jfb updated the diff for D79279: Add overloaded versions of builtin mem* functions.

Update overloading as discussed: on the original builtins, and separate the _sized variant, making its 4th parameter non-optional. If this looks good, I'll need to update codege for a few builtins (to handle volatile), as well as add tests for their codegen and address space (which should already work, but isn't tested).

Aug 13 2020, 10:03 AM · Restricted Project, Restricted Project
jfb added a comment to D85044: Add __atomic_is_lock_free to compiler-rt.

Can you please add tests for this?

Aug 13 2020, 9:22 AM · Restricted Project, Restricted Project, Restricted Project

Aug 12 2020

jfb added a comment to D85628: [HotColdSplitting] Add command line options for supplying cold function names via user input..
In D85628#2213940, @rjf wrote:
In D85628#2213919, @vsk wrote:

I’m not convinced this is a good idea. In what use case is it not possible to mark up relevant functions? It doesn’t make sense to me to make alternations to standard library functions within the compiler. It seems better to simply patch the standard library. In some cases llvm does infer function attributes for library functions, but these are generally lower level attributes that can’t be specified at the source level, and the attribute is made available to other passes in the pipeline.

Do you mean this patch isn't a good idea in general, or the recent revision isn't a good idea? For the latter, I'm not sure if you meant we should not outline declarations or we should not split the original loop into two (e.g. marking as cold before outlining). IMO splitting the loop into two simply addresses what the original intent of what we're doing, which is to mark certain functions as cold before outlining. Whereas, if we don't outline declarations via user-provided input, it renders @hiraditya 's proposed testcase useless. Alternatively, we don't have to make the testcase involving standard library functions if that's what you want :).

Aug 12 2020, 3:08 PM · Restricted Project
jfb added a comment to D85044: Add __atomic_is_lock_free to compiler-rt.

Does this x86 change actually match what the runtime does in the same file?

Aug 12 2020, 1:10 PM · Restricted Project, Restricted Project, Restricted Project

Aug 11 2020

jfb added a comment to D79279: Add overloaded versions of builtin mem* functions.

I think it would be reasonable in general to guarantee that our __builtin_ functions have contracts at least as wide as the underlying C function, but allow them to have extensions, and to keep the plain C functions unextended. I had actually thought we already did that in more cases than we do (but perhaps I was thinking about the LLVM math intrinsics that guarantee to not set errno). That would mean that a C standard library implementation is still free to #define foo(x,y,z) __builtin_foo(x,y,z), but if they do, they may pick up extensions.

Aug 11 2020, 11:17 AM · Restricted Project, Restricted Project

Aug 10 2020

jfb added a comment to D85691: lld: link libatomic if needed for Timer.
In D85691#2208323, @jfb wrote:

You're probably better off putting the std::chrono::nanoseconds::rep behind a lock if it's not always lock free on all platforms. Some platforms just don't have libatomic, so this patch might cause other issues.

Wouldn't such a platform get a fatal error in llvm/cmake/modules/CheckAtomic.cmake?

Aug 10 2020, 3:59 PM · Restricted Project
jfb added a comment to D85691: lld: link libatomic if needed for Timer.

You're probably better off putting the std::chrono::nanoseconds::rep behind a lock if it's not always lock free on all platforms. Some platforms just don't have libatomic, so this patch might cause other issues.

Aug 10 2020, 3:31 PM · Restricted Project
jfb added a comment to D82892: [NFC] Methods to compare IR added in each IR subclass.

I didn't check that all of these compare all the required state for all opcodes.

Aug 10 2020, 3:00 PM · Restricted Project

Aug 5 2020

jfb added a comment to D79279: Add overloaded versions of builtin mem* functions.

Two observations that are new to me in this review:

  1. We already treat all builtins as being overloaded on address space.
  2. The revised patch treats __builtin_mem*_overloaded as being overloaded *only* on address space, volatility, and atomicity. (We've tuned the design to a point where the other qualifiers don't matter any more.)

So, we're adding three features here: overloading on (a) address space, (b) volatility, and (c) atomicity. (a) is already available in the non-_overloaded form, and we seem to be approaching agreement that (b) should be available in the non-_overloaded form too. So that only leaves (c), which is really not _overloaded but _atomic.

Based on those observations I'm now thinking that we might prefer a somewhat different approach (but one that should require only minimal changes to the patch in front of us). Specifically:

  1. Stop treating lib builtins (eg, plain memcpy) as overloaded on address space. That's a (pre-existing) conformance bug, at least for the Embedded C TR.
  2. Keep __builtin_ forms of lib builtins overloaded on address space. (No change.)
  3. Also overload __builtin_ forms of lib builtins on volatility where it makes sense, instead of adding new builtin names __builtin_mem*_overloaded.
  4. Add a new name for the builtin for the atomic forms of memcpy and memset (__builtin_memcpy_unordered_atomic maybe?).
  5. Convert the "trivial types" check from an error to a warning and apply it to all the mem* overloads. (Though I think we might actually already have such a check, so this might only require extending it to cover the atomic builtin.)

What do you think?

Aug 5 2020, 3:02 PM · Restricted Project, Restricted Project
jfb updated the diff for D79279: Add overloaded versions of builtin mem* functions.

Remove restrict, update docs, call isCompleteType

Aug 5 2020, 3:02 PM · Restricted Project, Restricted Project
jfb added inline comments to D79279: Add overloaded versions of builtin mem* functions.
Aug 5 2020, 2:42 PM · Restricted Project, Restricted Project
jfb updated the diff for D79279: Add overloaded versions of builtin mem* functions.

Use 'access size' instead of 'element size'.

Aug 5 2020, 2:42 PM · Restricted Project, Restricted Project
jfb added inline comments to D79279: Add overloaded versions of builtin mem* functions.
Aug 5 2020, 2:37 PM · Restricted Project, Restricted Project
jfb updated the diff for D79279: Add overloaded versions of builtin mem* functions.

Add loop test requested by Vedant

Aug 5 2020, 2:36 PM · Restricted Project, Restricted Project
jfb updated the diff for D79279: Add overloaded versions of builtin mem* functions.

Do UBSan change suggested by Vedant.

Aug 5 2020, 2:02 PM · Restricted Project, Restricted Project
jfb added inline comments to D84947: Add libFuzzer shared object build output.
Aug 5 2020, 11:27 AM · Restricted Project, Restricted Project

Aug 4 2020

jfb added a comment to D79279: Add overloaded versions of builtin mem* functions.

Patch looks basically okay to me, although I'll second Richard's concern that we shouldn't absent-mindedly start producing overloaded memcpys for ordinary __builtin_memcpy.

Aug 4 2020, 9:21 PM · Restricted Project, Restricted Project
jfb updated the diff for D79279: Add overloaded versions of builtin mem* functions.

Update docs

Aug 4 2020, 9:21 PM · Restricted Project, Restricted Project
jfb added inline comments to D79279: Add overloaded versions of builtin mem* functions.
Aug 4 2020, 5:57 PM · Restricted Project, Restricted Project
jfb added a comment to D79279: Add overloaded versions of builtin mem* functions.

Thanks for the detailed comments, I think I've addressed all of them! I also added UBSan support to check the builtin invocation. I think this patch is pretty much ready to go. A follow-up will need to add the support functions to compiler-rt (they're currently optional, as per https://reviews.llvm.org/D33240), and in cases where size is known we can inline them (as we do for memcpy and friends).

Aug 4 2020, 5:51 PM · Restricted Project, Restricted Project
jfb updated the diff for D79279: Add overloaded versions of builtin mem* functions.

Address Richard's comments, add UBSan support.

Aug 4 2020, 5:49 PM · Restricted Project, Restricted Project
jfb accepted D85239: [DOCS] Add more detail to stack protector documentation.
Aug 4 2020, 1:33 PM · Restricted Project