Page MenuHomePhabricator

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

aganea (Alexandre Ganea)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 6 2017, 1:24 PM (302 w, 5 d)

Recent Activity

Fri, Sep 15

aganea abandoned D157136: [LLD][COFF] Handle 'label' symbols when they point to a COMDAT section.
Fri, Sep 15, 1:55 PM · Restricted Project, Restricted Project

Fri, Sep 8

aganea accepted D155579: [Windows] Avoid using FileIndex for unique IDs.

LGTM, thank you Martin!

Fri, Sep 8, 4:05 AM · Restricted Project, Restricted Project

Wed, Sep 6

aganea added a comment to D155579: [Windows] Avoid using FileIndex for unique IDs.

How does this behave in your testcase @aganea?

I don't see any difference between this patch and the baseline, so that's good news.

Wed, Sep 6, 7:38 AM · Restricted Project, Restricted Project

Tue, Aug 29

aganea added a comment to D157136: [LLD][COFF] Handle 'label' symbols when they point to a COMDAT section.

I'm also wondering if instead of this patch, we couldn't craft something simpler, an heuristic that "fixes" the section header and the section symbol. Usually they seems to come in a pair, .text$mn, .text$x or a trio if debug symbols are involved: .text$mn, .debug$S, .text$x. Opinions?

I think there is prior art for doing this for mingw, we do some convoluted stuff to associate .text$_Z3foov / .pdata$_Z3foov / .xdata$_Z3foov together, but IIRC it's not good for performance.

I think for the moment I'm inclined to do nothing, unless this is really high priority for some user. It sounds like we can recommend using llvm-objcopy in place of binutils objcopy as a possible solution.

Tue, Aug 29, 3:26 PM · Restricted Project, Restricted Project

Aug 22 2023

aganea added a comment to D155579: [Windows] Avoid using FileIndex for unique IDs.

You who might have more use of nontrivial Windows build scenarios - how much impact would it be to go all in on the new path canonicalization + hash approach, i.e. ditching the file index entirely? Performance wise I would believe that it would be no significant change to before. The only thing that matters probably is how it behaves wrt symlinks/hardlinks, if those are present and in use.

Aug 22 2023, 7:35 AM · Restricted Project, Restricted Project

Aug 21 2023

aganea added a comment to D155579: [Windows] Avoid using FileIndex for unique IDs.

Unfortunately in our case the situation is even worse than what Martin was suggesting. My timings are 2x slower after this patch than without. Tested with a stage2 clang-cl built with ThinLTO and all optimizations on.

Aug 21 2023, 10:41 AM · Restricted Project, Restricted Project

Aug 18 2023

aganea accepted D158279: clang: Make rewrite-includes-macros.cpp runnable on non-Win.

Sure. This was meant to catch an issue that only happened on Windows but it should work on all platforms.

Aug 18 2023, 7:38 AM · Restricted Project, Restricted Project
aganea added a comment to D155579: [Windows] Avoid using FileIndex for unique IDs.

Another option could be to add a debug opt to disable this new code path, and skip all the canonicalization and reliability check. It could be one way or the other; either enabled by default or disabled by default. But probably better to enable it by default (this new code path) and let people add the flag if they need more performance.

Aug 18 2023, 4:32 AM · Restricted Project, Restricted Project

Aug 15 2023

aganea added a comment to D155579: [Windows] Avoid using FileIndex for unique IDs.

@thieta IIRC you had a usecase where you scan through large numbers of files with LLVM code somewhere. Are you able to take this for a spin to make sure it doesn't affect the performance of your usecase too much?

Will do, but probably not some time real soon

@thieta Do you have time to try this out in the near future? I'd like to at least know of any potential performance impact before landing this.

Aug 15 2023, 5:52 AM · Restricted Project, Restricted Project

Aug 13 2023

aganea updated the summary of D157136: [LLD][COFF] Handle 'label' symbols when they point to a COMDAT section.
Aug 13 2023, 8:45 PM · Restricted Project, Restricted Project
aganea added a comment to D157136: [LLD][COFF] Handle 'label' symbols when they point to a COMDAT section.

Sorry I will answer to your comments a bit later. I was trying to understand the root issue, and I've managed to reproduce it locally.

Aug 13 2023, 8:44 PM · Restricted Project, Restricted Project

Aug 4 2023

aganea requested review of D157136: [LLD][COFF] Handle 'label' symbols when they point to a COMDAT section.
Aug 4 2023, 1:43 PM · Restricted Project, Restricted Project

Jun 19 2023

aganea added a comment to D119049: [LLD] Allow usage of LLD as a library.

Hello @srj ! Sorry for the late answer.

Jun 19 2023, 5:36 AM · Restricted Project, Restricted Project, Restricted Project
aganea added a reverting change for rGaa495214b39d: Revert "[LLD] Allow usage of LLD as a library": rG6f2e92c10ceb: Re-land [LLD] Allow usage of LLD as a library.
Jun 19 2023, 4:35 AM · Restricted Project, Restricted Project
aganea committed rG6f2e92c10ceb: Re-land [LLD] Allow usage of LLD as a library (authored by aganea).
Re-land [LLD] Allow usage of LLD as a library
Jun 19 2023, 4:35 AM · Restricted Project, Restricted Project
aganea closed D119049: [LLD] Allow usage of LLD as a library.
Jun 19 2023, 4:35 AM · Restricted Project, Restricted Project, Restricted Project

Jun 16 2023

aganea added a comment to D119049: [LLD] Allow usage of LLD as a library.

With binary files.

I see that

config.environment["LLD_SRC_DIR"] = config.lld_src_dir

sets the correct path, but ninja check-lld-unit still fails due to thisPath.append(getenv("LLD_SRC_DIR")); getting an empty string...

Jun 16 2023, 3:48 PM · Restricted Project, Restricted Project, Restricted Project
aganea updated the diff for D119049: [LLD] Allow usage of LLD as a library.

With double quotes in lit.cfg.py

Jun 16 2023, 3:43 PM · Restricted Project, Restricted Project, Restricted Project
aganea updated the diff for D119049: [LLD] Allow usage of LLD as a library.

With binary files.

Jun 16 2023, 3:15 PM · Restricted Project, Restricted Project, Restricted Project
aganea added a comment to D119049: [LLD] Allow usage of LLD as a library.
arc patch D119049
ninja -C /tmp/Rel check-lld-unit

gives me failures.

% grep LLVM_USE_RELATIVE_PATHS_IN_FILES /tmp/Rel/CMakeCache.txt
LLVM_USE_RELATIVE_PATHS_IN_FILES:BOOL=OFF
--
/tmp/Rel/tools/lld/unittests/AsLibELF/./LLDAsLibELFTests --gtest_filter=AsLib.ROCm
--
ld.lld: error: target emulation unknown: -m or at least one .o file required
Failed to link: /home/maskray/llvm/lld/unittests/AsLibELF/Inputs/kernel1.o
/home/maskray/llvm/lld/unittests/AsLibELF/ROCm.cpp:69: Failure
Value of: runLinker("%S/Inputs/kernel1.o")
  Actual: false
Expected: true
ld.lld: error: target emulation unknown: -m or at least one .o file required
Failed to link: /home/maskray/llvm/lld/unittests/AsLibELF/Inputs/kernel2.o
/home/maskray/llvm/lld/unittests/AsLibELF/ROCm.cpp:70: Failure
Value of: runLinker("%S/Inputs/kernel2.o")
  Actual: false
Jun 16 2023, 3:05 PM · Restricted Project, Restricted Project, Restricted Project

Jun 15 2023

aganea added a comment to D119049: [LLD] Allow usage of LLD as a library.

I've reproduced (on Linux) the issue mentioned above. This doesn't happen on Windows/MSVC because __FILE__ (used in the ROCm.cpp test) always expands to absolute paths when building LLVM, since -DLLVM_USE_RELATIVE_PATHS_IN_FILES requires -ffile-prefix-map which is unsupported on MSVC-cl/clang-cl. Instead, I've switched to using an environment variable LLD_SRC_DIR (which stores relative paths where applicable), see changes in ROCm.cpp and lld/test/Unit/lit.cfg.py.

Jun 15 2023, 2:36 PM · Restricted Project, Restricted Project, Restricted Project
aganea updated the diff for D119049: [LLD] Allow usage of LLD as a library.
Jun 15 2023, 2:36 PM · Restricted Project, Restricted Project, Restricted Project

Jun 14 2023

aganea added a comment to D119049: [LLD] Allow usage of LLD as a library.

Hello, I’m out this afternoon, is anybody able to disable this test until I can take a look at it please? Otherwise I’ll do it tonight.

Jun 14 2023, 12:40 PM · Restricted Project, Restricted Project, Restricted Project

Jun 13 2023

aganea added a comment to D119049: [LLD] Allow usage of LLD as a library.

This will now need a rebase to fix merge conflicts. This is a very desired improvement:)

A reminder that don't forget to test LLD_DEFAULT_LD_LLD_IS_MINGW

Jun 13 2023, 1:25 PM · Restricted Project, Restricted Project, Restricted Project
aganea committed rG2700da5fe28d: [LLD] Allow usage of LLD as a library (authored by aganea).
[LLD] Allow usage of LLD as a library
Jun 13 2023, 1:23 PM · Restricted Project, Restricted Project
aganea closed D119049: [LLD] Allow usage of LLD as a library.
Jun 13 2023, 1:23 PM · Restricted Project, Restricted Project, Restricted Project
aganea committed rGadcdc9cc3740: [LLD][COFF] Allow overwriting directives exports with cmd-line exports (authored by aganea).
[LLD][COFF] Allow overwriting directives exports with cmd-line exports
Jun 13 2023, 12:30 PM · Restricted Project
aganea added a comment to D119049: [LLD] Allow usage of LLD as a library.

Thank you for the thorough review @MaskRay !

Jun 13 2023, 12:30 PM · Restricted Project, Restricted Project, Restricted Project
aganea closed D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports.
Jun 13 2023, 12:29 PM · Restricted Project, Restricted Project
aganea added a comment to D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports.

Thank you for reviewing @alvinhochun @mstorsjo !

Jun 13 2023, 12:10 PM · Restricted Project, Restricted Project
aganea committed rG5039d364547c: [Support] Remove TaskQueue (authored by teemperor).
[Support] Remove TaskQueue
Jun 13 2023, 7:20 AM · Restricted Project, Restricted Project
aganea closed D86338: Remove TaskQueue.
Jun 13 2023, 7:20 AM · Restricted Project, Restricted Project

Jun 12 2023

aganea added a comment to D86338: Remove TaskQueue.

I do have commit access but this was part of some llvm/Support cleanup that I gave up on a long time ago, so I never merged this. Before merging this someone probably should check that this is still unused within LLVM...

Jun 12 2023, 12:01 PM · Restricted Project, Restricted Project
Herald added a project to D86338: Remove TaskQueue: Restricted Project.

Are you able to commit this or can I do it?

Jun 12 2023, 10:37 AM · Restricted Project, Restricted Project

Jun 6 2023

aganea added a comment to rGe1aa91b36325: [InstCombine] Use KnownBits::shl() in SimplifyDemandedBits().

Hello @nikic ! When building a two-stage LLVM build, I'm seeing an infinite loop that lead to SimplifyDemandedBits. I thought this might be related to your recent changes. I'm trying to bisect, but I don't have a fast machine, are you able to test on your end?

This is the SimplifyDemandedBits in SDAG rather than InstCombine, so I doubt it is related. Does your build maybe contain https://reviews.llvm.org/D127115? That one is known to cause stage2 hangs and has already been reverted.

Jun 6 2023, 9:01 AM · Restricted Project, Restricted Project

Jun 5 2023

aganea added a comment to rGe1aa91b36325: [InstCombine] Use KnownBits::shl() in SimplifyDemandedBits().

Hello @nikic ! When building a two-stage LLVM build, I'm seeing an infinite loop that lead to SimplifyDemandedBits. I thought this might be related to your recent changes. I'm trying to bisect, but I don't have a fast machine, are you able to test on your end?

Jun 5 2023, 4:14 PM · Restricted Project, Restricted Project

May 5 2023

aganea updated the diff for D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports.

Simplify .drectve section and fix tests for real.

May 5 2023, 8:16 AM · Restricted Project, Restricted Project

May 4 2023

aganea added inline comments to D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports.
May 4 2023, 3:35 PM · Restricted Project, Restricted Project
aganea updated the diff for D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports.

Re-apply changes & rebase.

May 4 2023, 3:35 PM · Restricted Project, Restricted Project
aganea committed rGcd3180296c10: [llvm-objdump][COFF] Skip empty export entries when dumping the export table (authored by aganea).
[llvm-objdump][COFF] Skip empty export entries when dumping the export table
May 4 2023, 11:29 AM · Restricted Project, Restricted Project
aganea closed D149610: [llvm-objdump][COFF] Skip empty entries when dumping the export table.
May 4 2023, 11:29 AM · Restricted Project, Restricted Project

May 3 2023

aganea updated the diff for D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports.

Rebase on D149610

May 3 2023, 10:55 AM · Restricted Project, Restricted Project
aganea added a comment to D149610: [llvm-objdump][COFF] Skip empty entries when dumping the export table.

@hans @alvinhochun The rationale is to simply remove noise from the output when there are sparse ordinals in the table. I'm not quite sure how useful is to keeps those empty entries, since they will be skipped over by the kernel anyway. [...]

I don't think we need to follow exactly the behaviour of dumpbin. Can you check what Binutils objdump does just for comparison?

objdump skips over empty values, as this patch does (using a test added by D149611):

Fair enough. Hiding completely empty entries may not be so bad then, given that two other tools do the same

The check in COFFObjectFile.cpp, L1567 checks if the RVA is in the output sections. If it's outside the sections, that entry isn't exporting anything, and there will be no dynamic linking at runtime.

Still, I don't quite agree with hiding any entries with non-zero RVA or non-empty names, even if they may not be linked at runtime. My opinion is that if an export entry has either, then it is likely they have been deliberately there for a reason or possibly created in error, so they should still be shown in the output for reference. (I think you can get such symbols using -export:zeroRvaExport=__ImageBase.)

So, I think the condition for skipping should only be RVA == 0 && Name.empty(). I would also consider doing the checks directly in the for loop without adding the ExportDirectoryEntryRef::isEmpty function. Just move move I->getSymbolNameearlier so you can check the RVA and the name before printing the entry.

May 3 2023, 10:53 AM · Restricted Project, Restricted Project
aganea updated the diff for D149610: [llvm-objdump][COFF] Skip empty entries when dumping the export table.

As suggested.

May 3 2023, 10:53 AM · Restricted Project, Restricted Project
aganea committed rG72f6ea650e1d: [llvm-objdump][COFF] Keep columns aligned in the console output when exports… (authored by aganea).
[llvm-objdump][COFF] Keep columns aligned in the console output when exports…
May 3 2023, 6:56 AM · Restricted Project, Restricted Project
aganea committed rG14220fedbd9d: [LLD][COFF] Fix incorrect pattern in test (authored by aganea).
[LLD][COFF] Fix incorrect pattern in test
May 3 2023, 6:56 AM · Restricted Project

May 2 2023

aganea added inline comments to D149610: [llvm-objdump][COFF] Skip empty entries when dumping the export table.
May 2 2023, 11:56 AM · Restricted Project, Restricted Project
aganea added a comment to D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports.

I'm not quite sure how the parenting works in Phabricator I might be doing things wrong.

May 2 2023, 11:34 AM · Restricted Project, Restricted Project
aganea updated the diff for D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports.

Address comments.

May 2 2023, 11:34 AM · Restricted Project, Restricted Project
aganea added a comment to D149610: [llvm-objdump][COFF] Skip empty entries when dumping the export table.

@hans @alvinhochun The rationale is to simply remove noise from the output when there are sparse ordinals in the table. I'm not quite sure how useful is to keeps those empty entries, since they will be skipped over by the kernel anyway. The check in COFFObjectFile.cpp, L1567 checks if the RVA is in the output sections. If it's outside the sections, that entry isn't exporting anything, and there will be no dynamic linking at runtime.

May 2 2023, 10:47 AM · Restricted Project, Restricted Project
aganea committed rG8efc7de0e639: [llvm][unittests] Silence warning on MSVC after 5b2423183cb3 (authored by aganea).
[llvm][unittests] Silence warning on MSVC after 5b2423183cb3
May 2 2023, 5:50 AM · Restricted Project, Restricted Project
aganea closed D149609: Silence warning on MSVC after 5b2423183cb3.
May 2 2023, 5:49 AM · Restricted Project, Restricted Project

May 1 2023

aganea requested review of D149611: [LLD][COFF] Allow overwriting directives exports with cmd-line exports.
May 1 2023, 12:49 PM · Restricted Project, Restricted Project
aganea requested review of D149610: [llvm-objdump][COFF] Skip empty entries when dumping the export table.
May 1 2023, 12:39 PM · Restricted Project, Restricted Project
aganea requested review of D149609: Silence warning on MSVC after 5b2423183cb3.
May 1 2023, 12:33 PM · Restricted Project, Restricted Project

Apr 27 2023

aganea accepted D149268: [LLD][COFF] Fix PDB relocation on big-endian hosts.

LGTM, thanks for the update!

Apr 27 2023, 8:29 AM · Restricted Project, Restricted Project
aganea added inline comments to D149268: [LLD][COFF] Fix PDB relocation on big-endian hosts.
Apr 27 2023, 8:05 AM · Restricted Project, Restricted Project
aganea added a reviewer for D149268: [LLD][COFF] Fix PDB relocation on big-endian hosts: mstorsjo.
Apr 27 2023, 6:53 AM · Restricted Project, Restricted Project

Apr 26 2023

aganea added inline comments to D149268: [LLD][COFF] Fix PDB relocation on big-endian hosts.
Apr 26 2023, 11:32 AM · Restricted Project, Restricted Project

Apr 23 2023

aganea added inline comments to D140949: [MemProf] Context disambiguation cloning pass [patch 2/3].
Apr 23 2023, 5:55 PM · Restricted Project, Restricted Project

Apr 22 2023

aganea added inline comments to D140949: [MemProf] Context disambiguation cloning pass [patch 2/3].
Apr 22 2023, 6:49 PM · Restricted Project, Restricted Project
aganea added inline comments to D140949: [MemProf] Context disambiguation cloning pass [patch 2/3].
Apr 22 2023, 2:41 PM · Restricted Project, Restricted Project

Apr 12 2023

aganea updated subscribers of D147256: [DebugInfo] Fix file path separator when targeting windows..

+ @saudi @thieta

Apr 12 2023, 12:27 PM · Restricted Project, Restricted Project, Restricted Project

Apr 3 2023

aganea accepted D147340: [Support] Improve Windows performance of buffered raw_ostream.

Sounds good!
I'm just curious, have you tried experimenting with different sizes? Would it make sense to set a higher value for the mapfile using .SetBufferSize()?

Apr 3 2023, 6:17 AM · Restricted Project, Restricted Project

Apr 2 2023

aganea updated subscribers of rGceff9524f924: [mlir][StorageUniquer] Fix build with LLVM_ENABLE_THREADS=OFF.

+ @rriddle

Apr 2 2023, 8:40 AM · Restricted Project, Restricted Project
aganea committed rGceff9524f924: [mlir][StorageUniquer] Fix build with LLVM_ENABLE_THREADS=OFF (authored by aganea).
[mlir][StorageUniquer] Fix build with LLVM_ENABLE_THREADS=OFF
Apr 2 2023, 8:39 AM · Restricted Project, Restricted Project

Mar 24 2023

aganea accepted D146796: [llvm-rc] Fix the reference to the option for disabling preprocessing in a message.

LG.

Mar 24 2023, 7:18 AM · Restricted Project, Restricted Project
aganea accepted D146794: [llvm-rc] Look for "clang-<major>" when locating a suitable preprocessor.

LG.

Mar 24 2023, 7:17 AM · Restricted Project, Restricted Project
aganea accepted D146797: [llvm-rc] Remove transitional preprocessing fallback logic.

Just a thought: as future work, we should probably have an integration with LLVM_TOOL_LLVM_DRIVER_BUILD?

Mar 24 2023, 7:14 AM · Restricted Project, Restricted Project
aganea accepted D146793: [llvm-rc] Respect the executable specified in the --preprocessor command.

Something was missing in the description:

The arguments passed in this option were passed onto the child
process, but we still blindly [used] the clang binary that we had found
to sys::ExecuteAndWait as the intended executable to run.

Mar 24 2023, 7:05 AM · Restricted Project, Restricted Project

Mar 21 2023

aganea added a comment to D146490: [Support] On Windows, ensure that UniqueID is really stable.

Fair enough. There are several choices forward: either we mark the issue as "Will Not Fix" or I can try only scoping this patch to only keep the handle open for network drives/paths. Any other suggestions?

Mar 21 2023, 6:33 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Mar 20 2023

aganea updated the diff for D146490: [Support] On Windows, ensure that UniqueID is really stable.
Mar 20 2023, 7:32 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aganea requested review of D146490: [Support] On Windows, ensure that UniqueID is really stable.
Mar 20 2023, 7:17 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Mar 15 2023

aganea committed rG7adaefbaa8bd: [wasm] Silence 'not all control paths return a value' warning when (authored by aganea).
[wasm] Silence 'not all control paths return a value' warning when
Mar 15 2023, 11:20 AM · Restricted Project
aganea committed rGff7dfe4c403c: [DebugInfo][MSF] Silence 'not all control paths return a value' warning (authored by aganea).
[DebugInfo][MSF] Silence 'not all control paths return a value' warning
Mar 15 2023, 11:20 AM · Restricted Project, Restricted Project

Mar 12 2023

aganea added a comment to D145873: Move definitions of ArgKind from Intrinsics.h to Intrinsics.td.

Reid is out for vacations, I’d suggest perhaps to find another reviewer!

Mar 12 2023, 5:13 PM · Restricted Project, Restricted Project

Feb 24 2023

aganea added inline comments to D142224: [Support] Emulate SIGPIPE handling in raw_fd_ostream write for Windows.
Feb 24 2023, 12:05 PM · Restricted Project, Restricted Project, Restricted Project

Feb 22 2023

aganea accepted D144385: [PDB] Error on too large stream directories.

Thanks for the changes!

Feb 22 2023, 10:58 AM · Restricted Project, Restricted Project

Feb 20 2023

aganea committed rG1d0a5f11c04e: [Support] Silence warning with Clang ToT. (authored by aganea).
[Support] Silence warning with Clang ToT.
Feb 20 2023, 3:25 PM · Restricted Project, Restricted Project
aganea added inline comments to D119049: [LLD] Allow usage of LLD as a library.
Feb 20 2023, 3:07 PM · Restricted Project, Restricted Project, Restricted Project
aganea updated the diff for D119049: [LLD] Allow usage of LLD as a library.

As sugested by @MaskRay

Feb 20 2023, 3:07 PM · Restricted Project, Restricted Project, Restricted Project
aganea added a comment to D115103: Leak Sanitizer port to Windows.

@clemenswasser We hit this once in a while, when Microsoft update their libraries. But I'm surprised about the CC at the beginning. How can this issue be reproduced? Can you show a disassembly dump from the VS debugger, around the address of the function that fails?

Feb 20 2023, 11:50 AM · Restricted Project, Restricted Project, Restricted Project
aganea added a comment to D144385: [PDB] Error on too large stream directories.

In addition to these changes, do you think you can add a more descriptive error at a higher level in LLD, maybe in PDBLinker::commit(). It is not clear from the message what the corrective measure is. I suppose adding /pdbpagesize: with a higher block size would fix the problem, but this isn't obvious to everyone.

Feb 20 2023, 11:45 AM · Restricted Project, Restricted Project

Feb 17 2023

aganea added a comment to D119049: [LLD] Allow usage of LLD as a library.

Thanks for the in-depth review @MaskRay, I appreciate!

Feb 17 2023, 5:19 PM · Restricted Project, Restricted Project, Restricted Project
aganea updated the diff for D119049: [LLD] Allow usage of LLD as a library.

As suggested by @MaskRay

Feb 17 2023, 5:19 PM · Restricted Project, Restricted Project, Restricted Project

Feb 8 2023

aganea committed rGf8ea2f6e93c1: [Support] Clarify CrashRecoveryContext exception codes on Windows. NFC (authored by aganea).
[Support] Clarify CrashRecoveryContext exception codes on Windows. NFC
Feb 8 2023, 3:04 PM · Restricted Project, Restricted Project
aganea closed D143609: [Support] Clarify CrashRecoveryContext exception codes on Windows.
Feb 8 2023, 3:04 PM · Restricted Project, Restricted Project
aganea requested review of D143609: [Support] Clarify CrashRecoveryContext exception codes on Windows.
Feb 8 2023, 2:55 PM · Restricted Project, Restricted Project
aganea added inline comments to D142224: [Support] Emulate SIGPIPE handling in raw_fd_ostream write for Windows.
Feb 8 2023, 8:52 AM · Restricted Project, Restricted Project, Restricted Project

Feb 5 2023

aganea updated the diff for D119049: [LLD] Allow usage of LLD as a library.

As suggested by @MaskRay. This changes a few things, but it's for the best I think:

Feb 5 2023, 2:37 PM · Restricted Project, Restricted Project, Restricted Project
aganea added a comment to D142949: [lld] Destroy CommonLinkerContext inside lld::*::link after D108850.

How is auto *ctx = new CommonLinkerContext (with a later cleanup) different from an on-stack COFFLinkerContext ctx; whose destructor is not called?

How is the destructor not called in this patch? ctx on the stack in this patch will call ~CommonLinkerContext() and the would destroy all the instances, the string saver, the bump allocator, etc. We want to avoid that in the "normal" scenario, for a quick exit.

Feb 5 2023, 8:11 AM · Restricted Project, Restricted Project, Restricted Project

Feb 1 2023

aganea added a comment to D115103: Leak Sanitizer port to Windows.

Do you have a suggestion how I could fix this on MSVC

Feb 1 2023, 12:02 PM · Restricted Project, Restricted Project, Restricted Project
aganea added a comment to D142224: [Support] Emulate SIGPIPE handling in raw_fd_ostream write for Windows.

Sorry for the spam. Test coverage would be welcome too :-) The following should exercise the crash: RUN: llvm-readobj --help | head -c1

Feb 1 2023, 11:34 AM · Restricted Project, Restricted Project, Restricted Project
aganea added a comment to D142224: [Support] Emulate SIGPIPE handling in raw_fd_ostream write for Windows.

You'd probably need to remove this #define: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/Driver.cpp#L1876

Feb 1 2023, 10:59 AM · Restricted Project, Restricted Project, Restricted Project

Jan 31 2023

aganea added a comment to D142949: [lld] Destroy CommonLinkerContext inside lld::*::link after D108850.

Do you happen to have more information about which specific line may trigger a problem on Windows? Does this patch regress it?

The previous issue was caused by this call to fatal but you fixed it in rGd9dbf9e30a581fcadd667b6d8e5827a4003b85a2, see problem description in rG45b8a741fbbf.
My whole point is that we don't need this current patch, users can just call lld::safeLldMain().

Jan 31 2023, 1:37 PM · Restricted Project, Restricted Project, Restricted Project
aganea added a comment to D142547: [LLD] cleans up context and symbol table to allow multiple invocations to lld::elf::linker().

As mentioned in https://github.com/llvm/llvm-project/issues/59874, this is on purpose. More info here: https://reviews.llvm.org/D142949#4093346. One should call safeLldMain() instead! Let me finish/land D119049 and things should look better afterwards.

Jan 31 2023, 1:32 PM · Restricted Project, lld, Restricted Project
aganea added a comment to D142949: [lld] Destroy CommonLinkerContext inside lld::*::link after D108850.

The reason for not allocating ctx on the stack is to survive calls to lld::error() in LLD-as-lib scenarios.

Just another quick note -- in theory we can survive crashes as well here. But there's at least a case in lldELF which throws an error() inside a constructor, and that corrupts the heap on ctx deletion, on Windows at least, see rG45b8a741fbbf271e0fb71294cb7cdce3ad4b9bf3

Jan 31 2023, 5:35 AM · Restricted Project, Restricted Project, Restricted Project
aganea added a comment to D142949: [lld] Destroy CommonLinkerContext inside lld::*::link after D108850.

The OP in PR59874 could use safeLldMain instead of lld::elf::link and that will solve their issue (but lld.cpp.obj has to be linked in). With D119049 we should probably decide if we want to expose the naked lld::*::link functions outside of LLD, or if users should always use safeLldMain, or something else.

Jan 31 2023, 4:45 AM · Restricted Project, Restricted Project, Restricted Project

Jan 26 2023

aganea added a comment to D119049: [LLD] Allow usage of LLD as a library.

Thanks for reviewing @jerryyin !
Before going further, I'd like an review/approval from the regular lld contributors as well, whenever you got the chance @mstorsjo @MaskRay @hans @thieta

Jan 26 2023, 6:59 AM · Restricted Project, Restricted Project, Restricted Project

Jan 24 2023

aganea updated the diff for D119049: [LLD] Allow usage of LLD as a library.

As suggested by @arsenm

Jan 24 2023, 6:36 PM · Restricted Project, Restricted Project, Restricted Project