inglorion (Bob Haarman)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 15 2015, 10:55 AM (135 w, 2 d)

Recent Activity

Tue, Jul 17

inglorion committed rL337344: Revert "[InstCombine] Fold 'check for [no] signed truncation' pattern".
Revert "[InstCombine] Fold 'check for [no] signed truncation' pattern"
Tue, Jul 17, 7:23 PM

Mon, Jul 9

inglorion committed rC336604: Added -fcrash-diagnostics-dir flag.
Added -fcrash-diagnostics-dir flag
Mon, Jul 9, 2:12 PM
inglorion committed rL336604: Added -fcrash-diagnostics-dir flag.
Added -fcrash-diagnostics-dir flag
Mon, Jul 9, 2:12 PM
inglorion closed D48601: Added -fcrash-diagnostics-dir flag.
Mon, Jul 9, 2:12 PM

Thu, Jun 28

inglorion committed rL335864: lld-link: align sections to 16 bytes if referenced from the gfids table.
lld-link: align sections to 16 bytes if referenced from the gfids table
Thu, Jun 28, 8:27 AM
inglorion committed rLLD335864: lld-link: align sections to 16 bytes if referenced from the gfids table.
lld-link: align sections to 16 bytes if referenced from the gfids table
Thu, Jun 28, 8:27 AM
inglorion closed D48690: lld-link: align sections to 16 bytes if referenced from the gfids table.
Thu, Jun 28, 8:27 AM
inglorion updated the diff for D48690: lld-link: align sections to 16 bytes if referenced from the gfids table.

improvements suggested by @ruiu and @hans (thanks!)

Thu, Jun 28, 8:24 AM
inglorion added inline comments to D48690: lld-link: align sections to 16 bytes if referenced from the gfids table.
Thu, Jun 28, 8:20 AM

Wed, Jun 27

inglorion updated the diff for D48690: lld-link: align sections to 16 bytes if referenced from the gfids table.

simplified as suggested by @pcc (thanks!)

Wed, Jun 27, 6:03 PM
inglorion added inline comments to D48690: lld-link: align sections to 16 bytes if referenced from the gfids table.
Wed, Jun 27, 5:40 PM
inglorion created D48690: lld-link: align sections to 16 bytes if referenced from the gfids table.
Wed, Jun 27, 5:20 PM

Jun 14 2018

inglorion added a comment to D47540: [lld] Enable Visual Studio compatible output.

I think this presents a choice that could affect the implementation. If all the call sites are converted, then potentially we could make the Src parameter non-optional.

Jun 14 2018, 11:39 AM

Jun 13 2018

inglorion added a comment to D47540: [lld] Enable Visual Studio compatible output.

finds a lot of places where we are putting location information in diagnostics. Are you planning to convert those to use the new API/formatting, too?

Jun 13 2018, 3:44 PM
inglorion added inline comments to D47540: [lld] Enable Visual Studio compatible output.
Jun 13 2018, 3:39 PM
inglorion added inline comments to D47540: [lld] Enable Visual Studio compatible output.
Jun 13 2018, 3:08 PM
inglorion added a reviewer for D47540: [lld] Enable Visual Studio compatible output: inglorion.
Jun 13 2018, 2:54 PM
inglorion abandoned D48104: [lld] add ability to register exit handlers.

We decided against shipping the feature that depends on this. Given that, I'd rather not have this code. Abandoning the diff.

Jun 13 2018, 2:53 PM
inglorion added a comment to D48104: [lld] add ability to register exit handlers.

Thanks for your suggestions, sbc. Those would be good to implement if we wanted the feature. However, we decided against the feature that depends on this, so I'll be abandoning this diff.

Jun 13 2018, 2:52 PM
inglorion abandoned D47799: [COFF] add /errorrepro to save reproducer on error.

After checking with ruiu, it seems we are all on the side of not adding this. Abandoning.

Jun 13 2018, 2:50 PM
inglorion accepted D48051: LTO: Keep file handles open for memory mapped files..

LGTM, thanks!

Jun 13 2018, 9:54 AM

Jun 12 2018

inglorion added a comment to D47799: [COFF] add /errorrepro to save reproducer on error.

I still don't see a compelling reason to do this to be honest.

Jun 12 2018, 5:38 PM
inglorion added a dependent revision for D48104: [lld] add ability to register exit handlers: D47799: [COFF] add /errorrepro to save reproducer on error.
Jun 12 2018, 3:53 PM
inglorion added a dependency for D47799: [COFF] add /errorrepro to save reproducer on error: D48104: [lld] add ability to register exit handlers.
Jun 12 2018, 3:53 PM
inglorion updated the diff for D47799: [COFF] add /errorrepro to save reproducer on error.

Re-upload.

Jun 12 2018, 3:52 PM
inglorion updated the diff for D47799: [COFF] add /errorrepro to save reproducer on error.

Ensure reproducer is also written when calling exitLld().

Jun 12 2018, 3:51 PM
inglorion created D48104: [lld] add ability to register exit handlers.
Jun 12 2018, 3:50 PM
inglorion added inline comments to D47799: [COFF] add /errorrepro to save reproducer on error.
Jun 12 2018, 3:26 PM
inglorion added a comment to D48051: LTO: Keep file handles open for memory mapped files..

Alright, sounds like we are in agreement about the code. @pcc, can you still change the wording from "bug" to something else (like "observed behavior")?

Jun 12 2018, 1:13 PM

Jun 11 2018

inglorion added a comment to D48051: LTO: Keep file handles open for memory mapped files..

Thanks for the fix! A couple of suggestions:

Jun 11 2018, 2:56 PM
inglorion added a reviewer for D48051: LTO: Keep file handles open for memory mapped files.: inglorion.
Jun 11 2018, 2:40 PM

Jun 8 2018

inglorion accepted D46205: Set MemoryBuffer's RequiresNullTerminator to false by default..

Alright then, LGTM.

Jun 8 2018, 5:04 PM
inglorion added a comment to D46205: Set MemoryBuffer's RequiresNullTerminator to false by default..

I haven't accepted the patch yet because I want to make sure we do end up removing the RequiresNullTerminator flag. You indicated that you don't want to do that in this patch. That's ok with me as long as you do plan to do it in a soon-to-come follow-up patch. If you don't plan to write that follow-up patch, I would like you to include the new function and rewrite the calls in this patch.

Jun 8 2018, 4:26 PM
inglorion added a comment to D46205: Set MemoryBuffer's RequiresNullTerminator to false by default..

Thanks for doing this! I did some of the ground work for this in the COFF linker, but never got around to flipping the default. I too like the idea of having a separate function for the NUL-terminated case. It seems we all want that, so how do we get there? Are you planning to follow this up with a patch that removes the RequiresNullTerminator flag and rewrites calls that use it to use the new function instead?

Jun 8 2018, 12:58 PM

Jun 6 2018

inglorion committed rL334154: [COFF] report file containing unsupported relocation.
[COFF] report file containing unsupported relocation
Jun 6 2018, 5:54 PM
inglorion committed rLLD334154: [COFF] report file containing unsupported relocation.
[COFF] report file containing unsupported relocation
Jun 6 2018, 5:54 PM
inglorion closed D45911: [COFF] report file containing unsupported relocation.
Jun 6 2018, 5:54 PM
inglorion added inline comments to D47799: [COFF] add /errorrepro to save reproducer on error.
Jun 6 2018, 11:45 AM
inglorion added a comment to D47799: [COFF] add /errorrepro to save reproducer on error.

Thanks, @pcc . Out of curiosity, how did you build chrome (I guess I'm mostly interested in args.gn and the command you used to build). I've been trying this on my machine and it can literally go for weeks without hitting the problem.

Jun 6 2018, 11:34 AM

Jun 5 2018

inglorion added a reviewer for D45911: [COFF] report file containing unsupported relocation: rnk.
Jun 5 2018, 3:01 PM
inglorion added a comment to D47799: [COFF] add /errorrepro to save reproducer on error.

I am investigating https://bugs.chromium.org/p/chromium/issues/detail?id=786127, which is a failure that does not reproduce reliably. I could use /linkrepro to save all link inputs always, but that would use up a lot of disk space. Running the command again with /linkrepro will almost certainly not reproduce the failure. Having a way to only save the reproducer when an error occurs would help here, and also seems a generally useful feature to have.

Jun 5 2018, 2:42 PM
inglorion created D47799: [COFF] add /errorrepro to save reproducer on error.
Jun 5 2018, 2:25 PM

May 9 2018

inglorion abandoned D46214: Avoid reading past end of archive looking for long file name.

We landed D46527 instead.

May 9 2018, 2:58 PM
inglorion planned changes to D46621: [Support] call FlushFileBuffers when closing raw_fd_ostream on Windows.

Bruce's blog post (https://randomascii.wordpress.com/2018/02/25/compiler-bug-linker-bug-windows-kernel-bug/) does seem to suggest that this issue should only ever be hit when files are written through memory maps. If that is the case, this change should not make a difference. I'll shelve it for now and see if I can think of something better. Thanks!

May 9 2018, 10:50 AM

May 8 2018

inglorion added a comment to D46621: [Support] call FlushFileBuffers when closing raw_fd_ostream on Windows.

It's a speculative fix. I'm not getting the error locally and it doesn't consistently reproduce anywhere I know of, which makes it hard to think of a way to ascertain that it actually fixed the problem. It's just that I noticed that a number of files we write when doing ThinLTO are written through raw_fd_ostream, and we then occasionally see bytes being read as 0 when they shouldn't be, which is a problem we fixed elsewhere by calling FlushFileBuffers. If we land this and the problem persists I think we should revert it (for the reasons zturner mentioned), but given the similarity to the problem we worked around with D42925, I would like to give this a shot and see if it helps.

May 8 2018, 8:00 PM
inglorion added a comment to D46621: [Support] call FlushFileBuffers when closing raw_fd_ostream on Windows.

My hope is that this will help with https://crbug.com/786127. I haven't seen the undefined symbol errors in local builds for several weeks now, but they still happen on the bots.

May 8 2018, 7:31 PM
inglorion created D46621: [Support] call FlushFileBuffers when closing raw_fd_ostream on Windows.
May 8 2018, 7:27 PM
inglorion added a comment to D46527: Object: Find terminator correctly when getting long filenames from GNU archives.

Digging a bit deeper into this, I don't understand how this breaks. The crash indicates that we're running off the end of the file while reading from the long file name table while looking for a newline that isn't there. However, we seem to always create archives with the long file name table before any member that references it, and both GNU ar and llvm-ar will reject archives that are not structured that way. This means that there must be at least one member after the long file name table. Since the record at the beginning of a member ends in a newline (see ArchiveWriter::printRestOfMemberHeader), this means there must always be a newline between the end of the long file name table and the end of the file. For archives we write, every long file name including the last is followed by a newline (see ArchiveWriter::addToStringTable), so the newline we find should always be in memory we can read and always be at the end of the long file name we're looking at.

May 8 2018, 5:08 PM
inglorion added a comment to D46527: Object: Find terminator correctly when getting long filenames from GNU archives.

Thanks, hans. I spent all of yesterday trying to cook up a test case for this. Taking into account the code in MemoryBuffer, I created a file of 20480 (4096 * 5) bytes to get it mmapped and a multiple of the page size, but it did not reproduce the problem. Just in case, I also tried a file not a multiple of the page size, and a small file. No luck. I tried just building the targets that were failing on the bot, but they built just fine locally. I tried using the exact Clang and Chromium revisions that the bot used, but, again, things worked fine locally. I tried the revisions I used a week ago, when I was able to get a repro, but no luck with that either. I also installed afl to try and fuzz my way to something that exercises the failure mode. I'll run that today and see if that gets me something we can use as a test case. At least we have the fix in now.

May 8 2018, 10:26 AM
inglorion accepted D46527: Object: Find terminator correctly when getting long filenames from GNU archives.

Accepted so that you can land the fix if still needed, per my comment at https://bugs.chromium.org/p/chromium/issues/detail?id=840260#c9

May 8 2018, 12:55 AM

Apr 28 2018

inglorion added a comment to D46214: Avoid reading past end of archive looking for long file name.

A test case for this would be an archive with a long file name at the end of the string table. It may have to be a specific size to tickle the problem I'm trying to fix here, to prevent MemoryBuffer or the OS from padding the archive with 0 bytes, which would avoid the problem.

Apr 28 2018, 10:56 AM
inglorion added inline comments to D46214: Avoid reading past end of archive looking for long file name.
Apr 28 2018, 10:53 AM

Apr 27 2018

inglorion created D46214: Avoid reading past end of archive looking for long file name.
Apr 27 2018, 2:58 PM

Apr 25 2018

inglorion committed rLLD330883: [COFF] more informative "broken object file" diagnostics.
[COFF] more informative "broken object file" diagnostics
Apr 25 2018, 4:37 PM
inglorion committed rL330883: [COFF] more informative "broken object file" diagnostics.
[COFF] more informative "broken object file" diagnostics
Apr 25 2018, 4:37 PM
inglorion closed D46090: [COFF] more informative "broken object file" diagnostics.
Apr 25 2018, 4:37 PM
inglorion updated the diff for D46090: [COFF] more informative "broken object file" diagnostics.

s/existant/existent/

Apr 25 2018, 3:36 PM
inglorion created D46090: [COFF] more informative "broken object file" diagnostics.
Apr 25 2018, 3:34 PM

Apr 24 2018

inglorion committed rLLD330786: [COFF] create MemoryBuffers without requiring NUL terminators.
[COFF] create MemoryBuffers without requiring NUL terminators
Apr 24 2018, 4:20 PM
inglorion committed rL330786: [COFF] create MemoryBuffers without requiring NUL terminators.
[COFF] create MemoryBuffers without requiring NUL terminators
Apr 24 2018, 4:20 PM
inglorion closed D45909: [COFF] create MemoryBuffers without requiring NUL terminators.
Apr 24 2018, 4:20 PM
inglorion added a comment to D45909: [COFF] create MemoryBuffers without requiring NUL terminators.

Anyone able to take a look at this?

Apr 24 2018, 9:38 AM
inglorion added a comment to D45911: [COFF] report file containing unsupported relocation.

@ruiu, did you get a chance to look at this?

Apr 24 2018, 9:38 AM

Apr 20 2018

inglorion created D45911: [COFF] report file containing unsupported relocation.
Apr 20 2018, 4:56 PM
inglorion created D45909: [COFF] create MemoryBuffers without requiring NUL terminators.
Apr 20 2018, 4:30 PM
inglorion committed rL330490: Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp.
Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp
Apr 20 2018, 3:19 PM
inglorion committed rLLD330490: Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp.
Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp
Apr 20 2018, 3:19 PM
inglorion closed D45789: Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp.
Apr 20 2018, 3:19 PM
inglorion updated the diff for D45789: Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp.

Fair enough. Changed it to only do the memcpy if the ArrayRef is not
empty, and otherwise leave the function alone.

Apr 20 2018, 2:53 PM

Apr 19 2018

inglorion updated the diff for D45789: Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp.

@ruiu, how about this?

Apr 19 2018, 1:30 PM

Apr 18 2018

inglorion created D45789: Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp.
Apr 18 2018, 4:33 PM
inglorion committed rL330301: Fix data race in X86FloatingPoint.cpp ASSERT_SORTED.
Fix data race in X86FloatingPoint.cpp ASSERT_SORTED
Apr 18 2018, 4:07 PM
inglorion closed D45742: Fix data race in X86FloatingPoint.cpp ASSERT_SORTED.
Apr 18 2018, 4:07 PM

Apr 17 2018

inglorion committed rL330236: Fix lock order inversion between ManagedStatic and Statistic.
Fix lock order inversion between ManagedStatic and Statistic
Apr 17 2018, 4:40 PM
inglorion closed D45398: Fix lock order inversion between ManagedStatic and Statistic.
Apr 17 2018, 4:40 PM
inglorion created D45742: Fix data race in X86FloatingPoint.cpp ASSERT_SORTED.
Apr 17 2018, 3:50 PM

Apr 12 2018

inglorion added inline comments to D45398: Fix lock order inversion between ManagedStatic and Statistic.
Apr 12 2018, 11:12 AM

Apr 9 2018

inglorion committed rL329637: [annotated_builder] try harder to clean build directories.
[annotated_builder] try harder to clean build directories
Apr 9 2018, 4:09 PM
inglorion closed D45345: [annotated_builder] try harder to clean build directories.
Apr 9 2018, 4:09 PM
inglorion added a comment to D45345: [annotated_builder] try harder to clean build directories.

Ah, my bad. Thanks for clarifying, @zturner.

Apr 9 2018, 4:01 PM
inglorion added a comment to D45345: [annotated_builder] try harder to clean build directories.

Doesn't shutil.rmtree handle this?

Apr 9 2018, 3:10 PM
inglorion added a comment to D45345: [annotated_builder] try harder to clean build directories.

Anyone able to take a look at this?

Apr 9 2018, 11:38 AM
inglorion updated the diff for D45398: Fix lock order inversion between ManagedStatic and Statistic.

slight reword of comment

Apr 9 2018, 11:37 AM
inglorion updated the diff for D45398: Fix lock order inversion between ManagedStatic and Statistic.

Thanks for looking, @dsanders. Does this comment make things clearer?

Apr 9 2018, 11:36 AM

Apr 6 2018

inglorion created D45398: Fix lock order inversion between ManagedStatic and Statistic.
Apr 6 2018, 6:16 PM

Apr 5 2018

inglorion created D45345: [annotated_builder] try harder to clean build directories.
Apr 5 2018, 6:32 PM
inglorion committed rL329365: report error messages in annotated_builder.py.
report error messages in annotated_builder.py
Apr 5 2018, 5:10 PM
inglorion closed D45295: report error messages in annotated_builder.py.
Apr 5 2018, 5:10 PM

Apr 4 2018

inglorion created D45295: report error messages in annotated_builder.py.
Apr 4 2018, 3:43 PM

Apr 3 2018

inglorion committed rLLD329088: [lld] fix data race in ELF/ICF.cpp.
[lld] fix data race in ELF/ICF.cpp
Apr 3 2018, 10:32 AM
inglorion committed rL329088: [lld] fix data race in ELF/ICF.cpp.
[lld] fix data race in ELF/ICF.cpp
Apr 3 2018, 10:32 AM
inglorion closed D45192: [lld] fix data race in ELF/ICF.cpp.
Apr 3 2018, 10:32 AM
inglorion closed D45192: [lld] fix data race in ELF/ICF.cpp.
Apr 3 2018, 10:32 AM
Herald added a reviewer for D45192: [lld] fix data race in ELF/ICF.cpp: espindola.

Thanks, ruiu. Added the newlines before committing.

Apr 3 2018, 10:32 AM

Apr 2 2018

inglorion created D45192: [lld] fix data race in ELF/ICF.cpp.
Apr 2 2018, 4:51 PM

Mar 26 2018

inglorion committed rLLD328610: [lld] fix data race in ICF.cpp.
[lld] fix data race in ICF.cpp
Mar 26 2018, 11:11 PM
inglorion committed rL328610: [lld] fix data race in ICF.cpp.
[lld] fix data race in ICF.cpp
Mar 26 2018, 11:11 PM
inglorion closed D44716: [lld] fix data race in ICF.cpp.
Mar 26 2018, 11:11 PM
inglorion added a comment to D44716: [lld] fix data race in ICF.cpp.

This now generates a components_unittests.exe that is the same size as without this change that works correctly. I also verified that the data race is gone on COFF. I will land this to give the bots a chance to chew on it, and then fix the same data race in the ELF linker.

Mar 26 2018, 11:09 PM
inglorion updated the diff for D44716: [lld] fix data race in ICF.cpp.

Fixed off-by-one error: LLVM's for_each_n takes an end index, not a count

Mar 26 2018, 6:17 PM