Page MenuHomePhabricator

guiand (Gui Andrade)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 9 2020, 10:09 AM (9 w, 2 d)

Recent Activity

Yesterday

guiand added a comment to D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

TBH, I don't see how this solves any problem. It just makes it a problem for someone in the future... (FWIW, I say this being in full support of noundef)

Wed, Aug 12, 11:43 AM · Restricted Project

Tue, Aug 11

guiand added a reviewer for D85797: Fix sigaction interceptor to always correctly populate oldact: eugenis.
Tue, Aug 11, 5:13 PM · Restricted Project
guiand updated the diff for D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Made the compiler flag non-public

Tue, Aug 11, 4:20 PM · Restricted Project, Restricted Project
guiand updated the diff for D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang.

Allows passing different extra arguments for different clang expansions

Tue, Aug 11, 4:18 PM · Restricted Project, Restricted Project
guiand updated the diff for D85573: [CGAtomic] Mark atomic libcall functions `nounwind`.

Added willreturn. I think nocapture is probably best left to a future change since it's more invasive -- it would require keeping tabs of which arguments to each function is a pointer arg.

Tue, Aug 11, 3:42 PM · Restricted Project
guiand requested review of D81678: Introduce noundef attribute at call sites for stricter poison analysis.

I think I'd like someone to take a look at the llvm-lit changes to see if this makes sense as an approach

Tue, Aug 11, 3:07 PM · Restricted Project, Restricted Project
guiand updated the diff for D81678: Introduce noundef attribute at call sites for stricter poison analysis.

To try to alleviate the tests issue, @eugenis and I discussed that it might be best to take it slow. So now this patch will mask off emitting the attribute on clang tests by default.

Tue, Aug 11, 3:06 PM · Restricted Project, Restricted Project
guiand added a comment to D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

After discussing with @eugenis, for the meantime it might be best to do the following:

  • Change the masking attribute to be -fdisable-noundef-analysis (name notwithstanding), and have it completely turn off all noundefs
  • Change the llvm-lit configuration to use the new codegen flag for all the tests by default
  • Have noundef emitted in the frontend by default (when the codegen flag isn't present)
Tue, Aug 11, 2:42 PM · Restricted Project
guiand requested review of D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang.
Tue, Aug 11, 2:35 PM · Restricted Project, Restricted Project
guiand resigned from D85768: [Utils] Add highlighting definition for byref IR attribute.

LGTM, but I don't really maintain this code.

Tue, Aug 11, 12:36 PM · Restricted Project

Mon, Aug 10

guiand added a comment to D85573: [CGAtomic] Mark atomic libcall functions `nounwind`.

I don't think we can necessarily guarantee argmemonly/readonly/writeonly, particularly since these library calls can take a lock somewhere inside. I'll definitely add nocapture and willreturn though.

Mon, Aug 10, 1:29 PM · Restricted Project
guiand committed rGc0b5000bd848: [MSAN RT] Use __sanitizer::mem_is_zero in __msan_test_shadow (authored by guiand).
[MSAN RT] Use __sanitizer::mem_is_zero in __msan_test_shadow
Mon, Aug 10, 12:23 PM
guiand closed D84961: [MSAN RT] Use __sanitizer::mem_is_zero in __msan_test_shadow.
Mon, Aug 10, 12:23 PM · Restricted Project
guiand updated the diff for D84961: [MSAN RT] Use __sanitizer::mem_is_zero in __msan_test_shadow.

Addressed comments

Mon, Aug 10, 12:05 PM · Restricted Project
guiand updated the diff for D85227: [Draft][MSAN] Cache stack traces and chained origins.

Addressed comments. For handling number of uses per stack trace, this uses a bit of a heuristic:

Mon, Aug 10, 10:14 AM · Restricted Project, Restricted Project

Fri, Aug 7

guiand added inline comments to D85559: [MSAN] Reintroduce libatomic load/store instrumentation.
Fri, Aug 7, 5:27 PM · Restricted Project, Restricted Project, Restricted Project
guiand updated the diff for D85559: [MSAN] Reintroduce libatomic load/store instrumentation.

Separated out the frontend change. Addressed other comments.

Fri, Aug 7, 5:26 PM · Restricted Project, Restricted Project, Restricted Project
guiand requested review of D85573: [CGAtomic] Mark atomic libcall functions `nounwind`.
Fri, Aug 7, 5:10 PM · Restricted Project
guiand updated the diff for D85559: [MSAN] Reintroduce libatomic load/store instrumentation.

Rebased on master (again)

Fri, Aug 7, 3:54 PM · Restricted Project, Restricted Project, Restricted Project
guiand updated the diff for D85559: [MSAN] Reintroduce libatomic load/store instrumentation.

Rebased on master

Fri, Aug 7, 3:52 PM · Restricted Project, Restricted Project, Restricted Project
guiand updated the diff for D85559: [MSAN] Reintroduce libatomic load/store instrumentation.

Simplified by returning to the old implementation, but having libatomic calls made nounwind (so we never see them as invokes).

Fri, Aug 7, 3:50 PM · Restricted Project, Restricted Project, Restricted Project
guiand added inline comments to D85559: [MSAN] Reintroduce libatomic load/store instrumentation.
Fri, Aug 7, 2:34 PM · Restricted Project, Restricted Project, Restricted Project
guiand requested review of D85559: [MSAN] Reintroduce libatomic load/store instrumentation.
Fri, Aug 7, 2:33 PM · Restricted Project, Restricted Project, Restricted Project
guiand added a reverting change for rG33d239513c88: [MSAN] Instrument libatomic load/store calls: rG17ff170e3a9b: Revert "[MSAN] Instrument libatomic load/store calls".
Fri, Aug 7, 12:48 PM
guiand committed rG17ff170e3a9b: Revert "[MSAN] Instrument libatomic load/store calls" (authored by guiand).
Revert "[MSAN] Instrument libatomic load/store calls"
Fri, Aug 7, 12:48 PM
guiand added a reverting change for D83337: [MSAN] Instrument libatomic load/store calls: rG17ff170e3a9b: Revert "[MSAN] Instrument libatomic load/store calls".
Fri, Aug 7, 12:47 PM · Restricted Project, Restricted Project

Thu, Aug 6

guiand updated the diff for D84961: [MSAN RT] Use __sanitizer::mem_is_zero in __msan_test_shadow.

Modified to just use __sanitizer::mem_is_zero

Thu, Aug 6, 6:20 PM · Restricted Project
guiand added a comment to D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

Most of the discussion has been in https://reviews.llvm.org/D81678 (implementing the attribute in clang) and https://reviews.llvm.org/D82316 (adding the attribute to langref).

Thu, Aug 6, 3:04 PM · Restricted Project
guiand added a comment to D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

At the very least, make whatever script you used to update these public, as I don't want to have to recreate it from scratch when merging this in. I had enough "fun" with the LLD mass-renaming (UpperCamel -> lowerCamel) and that was _with_ a supposedly-working script (it didn't quite do the right thing and I seem to recall there being two refactoring commits, only one of which had a script); I do not want a repeat of that experience.

Thu, Aug 6, 1:23 PM · Restricted Project
guiand added a comment to D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

Are you seriously adding an attribute to literally every argument and return value? Why is this the right representation?

Thu, Aug 6, 1:16 PM · Restricted Project

Wed, Aug 5

guiand accepted D85350: [msan] Support %ms in scanf..
Wed, Aug 5, 12:55 PM · Restricted Project
guiand added a comment to D85350: [msan] Support %ms in scanf..

LGTM. Does this support the a format specifier as well, or just m?

Wed, Aug 5, 12:55 PM · Restricted Project

Tue, Aug 4

guiand accepted D85259: [msan] Remove readnone and friends from call sites..

LGTM

Tue, Aug 4, 5:11 PM · Restricted Project
guiand added inline comments to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.
Tue, Aug 4, 11:53 AM · Restricted Project, Restricted Project
guiand updated the diff for D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

Integrated with Alias Analyzer, uses simpler mechanism for walking through BB and determining stores to alloca

Tue, Aug 4, 11:47 AM · Restricted Project, Restricted Project
guiand added inline comments to D85227: [Draft][MSAN] Cache stack traces and chained origins.
Tue, Aug 4, 10:37 AM · Restricted Project, Restricted Project
guiand requested review of D85227: [Draft][MSAN] Cache stack traces and chained origins.
Tue, Aug 4, 10:35 AM · Restricted Project, Restricted Project

Mon, Aug 3

guiand committed rGcaf002c7be44: [Utils] Add noundef attribute to vim/emacs/vscode syntax scripts (authored by guiand).
[Utils] Add noundef attribute to vim/emacs/vscode syntax scripts
Mon, Aug 3, 9:47 AM
guiand closed D84553: [Utils] Add noundef attribute to vim/emacs syntax scripts.
Mon, Aug 3, 9:46 AM · Restricted Project
guiand committed rG3ebd1ba64f3d: [MSAN] Instrument freeze instruction by clearing shadow (authored by guiand).
[MSAN] Instrument freeze instruction by clearing shadow
Mon, Aug 3, 9:42 AM
guiand closed D85040: [MSAN] Instrument freeze instruction by clearing shadow.
Mon, Aug 3, 9:42 AM · Restricted Project

Fri, Jul 31

guiand added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

It seems like collectInitializers leans heavily on the isPointerOffset function, which returns an offset if two pointers have a constant difference, nullopt if they don't. The problem here is that we can't distinguish isPointerOffset == nullopt happening because the offset is determined at runtime, or because the two pointers are completely unrelated.

Fri, Jul 31, 5:42 PM · Restricted Project, Restricted Project
guiand updated the diff for D85040: [MSAN] Instrument freeze instruction by clearing shadow.

Updated.

Fri, Jul 31, 3:38 PM · Restricted Project
guiand added a comment to D85040: [MSAN] Instrument freeze instruction by clearing shadow.

Oh, wait, the patch didn't update. One sec.

Fri, Jul 31, 3:38 PM · Restricted Project
guiand updated the diff for D85040: [MSAN] Instrument freeze instruction by clearing shadow.

Fixed to actually set the shadow and origin. Sorry for the mix-up!

Fri, Jul 31, 3:37 PM · Restricted Project
guiand updated the diff for D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

Rebased; all tests passing again. Removed the change to the ppc-*mmintrin.c tests, instead I just use the -disable-noundef-args flag`. Cleaned up typos.

Fri, Jul 31, 3:11 PM · Restricted Project
guiand added a comment to D85040: [MSAN] Instrument freeze instruction by clearing shadow.

Oh! I need to test that it's not checked as well.

Fri, Jul 31, 2:55 PM · Restricted Project
guiand added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

In general, this implementation looks pretty complex and easy to get wrong. I'd prefer something along the lines of AArch64StackTagging::collectInitializers - directly calculate the offset for each store/load instruction. It might do some extra work with unrelated memory instructions, but probably not too much.

Fri, Jul 31, 2:54 PM · Restricted Project, Restricted Project
guiand updated the diff for D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

Flattened some control flow, updated to properly use StoreOffs, and updated tests to cover chained GEPs

Fri, Jul 31, 2:43 PM · Restricted Project, Restricted Project
guiand requested review of D85040: [MSAN] Instrument freeze instruction by clearing shadow.
Fri, Jul 31, 11:55 AM · Restricted Project

Thu, Jul 30

guiand updated the diff for D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

I've cut it down to only within a basic block as @eugenis and @vitalybuka suggested.

Thu, Jul 30, 6:40 PM · Restricted Project, Restricted Project
guiand added inline comments to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.
Thu, Jul 30, 3:47 PM · Restricted Project, Restricted Project
guiand added inline comments to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.
Thu, Jul 30, 3:21 PM · Restricted Project, Restricted Project
guiand added inline comments to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.
Thu, Jul 30, 3:15 PM · Restricted Project, Restricted Project
guiand added a comment to D83595: [Draft][MSAN] Optimize away poisoning allocas that are always written before load.

This actually has very significant effects on some, but not all, benchmarks.

Thu, Jul 30, 11:56 AM · Restricted Project, Restricted Project
guiand requested review of D84961: [MSAN RT] Use __sanitizer::mem_is_zero in __msan_test_shadow.
Thu, Jul 30, 11:26 AM · Restricted Project
guiand added a comment to D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

@jdoerfert what would the procedure be for reviewing these test changes / getting this landed with the noundef patch?

Thu, Jul 30, 9:43 AM · Restricted Project

Wed, Jul 29

guiand abandoned D83499: [MSAN runtime] Add poison_stack function that also updates origin.

I agree, this probably isn't going to be worth it.

Wed, Jul 29, 11:41 AM · Restricted Project, Restricted Project
guiand added inline comments to D81678: Introduce noundef attribute at call sites for stricter poison analysis.
Wed, Jul 29, 11:18 AM · Restricted Project, Restricted Project
guiand updated the diff for D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Updated comment on disable-noundef-args option

Wed, Jul 29, 11:16 AM · Restricted Project, Restricted Project
guiand updated the diff for D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Addressed comments

Wed, Jul 29, 11:13 AM · Restricted Project, Restricted Project

Tue, Jul 28

guiand updated the diff for D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

All tests up to date. Of particular note are the ppc-*mmintrin.c tests, which seemed to drastically change upon rerunning the test autogen script.

Tue, Jul 28, 6:53 PM · Restricted Project
guiand updated the diff for D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Fix typo in MayDropFunctionReturn

Tue, Jul 28, 1:23 PM · Restricted Project, Restricted Project
guiand updated the diff for D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Fixes regression; allows emitting noundef for non-FunctionDecls as well.

Tue, Jul 28, 12:08 PM · Restricted Project, Restricted Project
guiand updated the diff for D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Incorporate C++'s more conservative checks for omitted return values

Tue, Jul 28, 11:46 AM · Restricted Project, Restricted Project
guiand accepted D84740: [Sanitizers] Test including rpc/xdr.h requires sunrpc.

LGTM. Thanks!

Tue, Jul 28, 9:17 AM · Restricted Project

Mon, Jul 27

guiand accepted D84510: [msan] Compile the libatomic.c test with a C compiler.

LGTM

Mon, Jul 27, 9:35 AM · Restricted Project

Fri, Jul 24

guiand updated the diff for D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Added an across-the-board test with several different interesting cases. @rsmith, I'm not sure how to test things like VTables/VTTs, since afaik the way they would be exposed to function attributes would be if a struct is being passed by value. But in that case, we currently never emit noundef.

Fri, Jul 24, 5:03 PM · Restricted Project, Restricted Project
guiand updated the diff for D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary.

Update tests to reflect more strict noundef rules

Fri, Jul 24, 2:36 PM · Restricted Project
Herald added a project to D84553: [Utils] Add noundef attribute to vim/emacs syntax scripts: Restricted Project.
Fri, Jul 24, 1:20 PM · Restricted Project
guiand committed rG1e77b3af125e: [MSAN] Allow inserting array checks (authored by guiand).
[MSAN] Allow inserting array checks
Fri, Jul 24, 1:13 PM
guiand closed D84446: [MSAN] Allow inserting array checks.
Fri, Jul 24, 1:13 PM · Restricted Project
guiand added a comment to D84510: [msan] Compile the libatomic.c test with a C compiler.

I think we can actually just drop the #include <stdatomic.h> line here. It was used for a previous version of the test but I forgot to remove it after some changes.

Fri, Jul 24, 9:27 AM · Restricted Project

Thu, Jul 23

guiand added inline comments to D84446: [MSAN] Allow inserting array checks.
Thu, Jul 23, 2:03 PM · Restricted Project
guiand added inline comments to D84446: [MSAN] Allow inserting array checks.
Thu, Jul 23, 1:37 PM · Restricted Project
guiand added a comment to D84446: [MSAN] Allow inserting array checks.

I don't believe this happens in C++ but wanted to add it for completeness's sake (or if some other LLVM based language might use it).

Thu, Jul 23, 12:41 PM · Restricted Project
Herald added a project to D84446: [MSAN] Allow inserting array checks: Restricted Project.
Thu, Jul 23, 9:57 AM · Restricted Project
guiand added inline comments to D82680: MSAN: Allow emitting checks for struct types.
Thu, Jul 23, 9:52 AM · Restricted Project
guiand committed rG3285b2424941: [MSAN] Allow emitting checks for struct types (authored by guiand).
[MSAN] Allow emitting checks for struct types
Thu, Jul 23, 9:51 AM
guiand closed D82680: MSAN: Allow emitting checks for struct types.
Thu, Jul 23, 9:51 AM · Restricted Project
guiand committed rG0025d52c0f24: [MSAN] Never allow checking calls to __sanitizer_unaligned_{load,store} (authored by guiand).
[MSAN] Never allow checking calls to __sanitizer_unaligned_{load,store}
Thu, Jul 23, 9:43 AM
guiand closed D84351: [MSAN] Never allow checking calls to __sanitizer_unaligned_{load,store}.
Thu, Jul 23, 9:43 AM · Restricted Project
guiand added a comment to rG33d239513c88: [MSAN] Instrument libatomic load/store calls.

@amyk Sorry about that! The issue should be fixed now.

Thu, Jul 23, 9:33 AM
guiand committed rG0edc13509920: [MSAN] Mark libatomic test unsupported on PowerPC (authored by guiand).
[MSAN] Mark libatomic test unsupported on PowerPC
Thu, Jul 23, 9:32 AM

Wed, Jul 22

guiand updated the diff for D84351: [MSAN] Never allow checking calls to __sanitizer_unaligned_{load,store}.

Added a test case

Wed, Jul 22, 4:30 PM · Restricted Project
guiand added a comment to D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Just saw your comment about tests as well. The idea was to have all tests ported over as part of a separate commit (I linked it in the main patch description) and then only to push either commit once both are ready to land.

Wed, Jul 22, 3:10 PM · Restricted Project, Restricted Project
guiand updated the summary of D81678: Introduce noundef attribute at call sites for stricter poison analysis.
Wed, Jul 22, 3:08 PM · Restricted Project, Restricted Project
guiand updated the diff for D81678: Introduce noundef attribute at call sites for stricter poison analysis.

Adds additional constraints on noundef: Not a nullptr_t, not a member pointer, and not coerced to a type of larger size. Disabled emitting in return position for non-C++ languages (or inside extern "C").

Wed, Jul 22, 3:00 PM · Restricted Project, Restricted Project
guiand updated the diff for D82680: MSAN: Allow emitting checks for struct types.

Reuploaded with context

Wed, Jul 22, 1:14 PM · Restricted Project
guiand updated the diff for D83427: [MSAN] Update tests due to widespread eager checking.

Sorry about that. Reuploaded with full context.

Wed, Jul 22, 1:11 PM · Restricted Project, Restricted Project
guiand updated the diff for D83427: [MSAN] Update tests due to widespread eager checking.

Kept only core of patch: adding new LIT configration for eager-checks

Wed, Jul 22, 11:22 AM · Restricted Project, Restricted Project
guiand added a comment to D84351: [MSAN] Never allow checking calls to __sanitizer_unaligned_{load,store}.

This patch actually has no dependency on clang emitting noundef, so it should be good to go.

Wed, Jul 22, 11:21 AM · Restricted Project
Herald added a project to D84351: [MSAN] Never allow checking calls to __sanitizer_unaligned_{load,store}: Restricted Project.
Wed, Jul 22, 11:20 AM · Restricted Project
guiand committed rGf93b55a5ab9d: [Sanitizers] Add interceptor for xdrrec_create (authored by guiand).
[Sanitizers] Add interceptor for xdrrec_create
Wed, Jul 22, 10:30 AM
guiand closed D83358: [Sanitizers] Add interceptor for xdrrec_create.
Wed, Jul 22, 10:30 AM · Restricted Project
guiand committed rG33d239513c88: [MSAN] Instrument libatomic load/store calls (authored by guiand).
[MSAN] Instrument libatomic load/store calls
Wed, Jul 22, 9:45 AM
guiand closed D83337: [MSAN] Instrument libatomic load/store calls.
Wed, Jul 22, 9:45 AM · Restricted Project, Restricted Project

Fri, Jul 17

guiand committed rG65936fed1490: [MSAN tests] Require android for sigandorset.cpp (authored by guiand).
[MSAN tests] Require android for sigandorset.cpp
Fri, Jul 17, 8:55 PM
guiand committed rG951584db4ffb: Revert "update libatomic instrumentation" (authored by guiand).
Revert "update libatomic instrumentation"
Fri, Jul 17, 8:55 PM
guiand added a reverting change for rG1f29171ae77f: update libatomic instrumentation: rG951584db4ffb: Revert "update libatomic instrumentation".
Fri, Jul 17, 8:54 PM