Page MenuHomePhabricator

ruiu (Rui Ueyama)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 18 2013, 12:16 AM (352 w, 3 d)

Recent Activity

Thu, Jan 16

ruiu added a comment to D72803: Make lld cmake not compute commit revision twice.

LGTM

Thu, Jan 16, 6:33 PM · Restricted Project

Thu, Jan 9

ruiu added a comment to D59780: Support Intel Control-flow Enforcement Technology.

@MaskRay Please rebase and upload it with me and other people as reviewers, then I'll LGTM.

Thu, Jan 9, 6:46 AM · Restricted Project

Mon, Jan 6

ruiu added inline comments to D72183: [ELF][PPC64] Add --lax-call-lacks-nop.
Mon, Jan 6, 12:53 AM · Restricted Project
ruiu added inline comments to D72183: [ELF][PPC64] Add --lax-call-lacks-nop.
Mon, Jan 6, 12:35 AM · Restricted Project

Sun, Jan 5

ruiu added inline comments to D72183: [ELF][PPC64] Add --lax-call-lacks-nop.
Sun, Jan 5, 10:43 PM · Restricted Project
ruiu accepted D72196: [lld] Fix trivial typos in comments.

LGTM

Sun, Jan 5, 10:34 PM · Restricted Project, lld
ruiu added inline comments to D71981: [LLD] [COFF] Don't error out on duplicate absolute symbols with the same value.
Sun, Jan 5, 10:25 PM · Restricted Project

Dec 19 2019

ruiu accepted D71735: [ELF] Don't suggest an alternative spelling for a symbol in a discarded section.

LGTM

Dec 19 2019, 9:46 PM · Restricted Project

Dec 18 2019

ruiu accepted D71679: [LLD] [COFF] Fix reporting duplicate errors for absolute symbols.

LGTM

Dec 18 2019, 8:02 PM · Restricted Project

Dec 17 2019

ruiu accepted D71631: [ELF] writePlt: replace parameters gotPltEntryAddr and index with `const Symbol &`. NFC.

I think I like sym over s as a function parameter name whose scope is not very narrow, but except that LGTM.

Dec 17 2019, 11:49 PM · Restricted Project
ruiu accepted D71101: [lld][RISCV] Use an e_flags of 0 if there are only binary input files..
Dec 17 2019, 6:36 PM · Restricted Project
ruiu added inline comments to D71632: [lld][WebAssembly] Fail if bitcode objects are pulled in after LTO.
Dec 17 2019, 6:27 PM · Restricted Project

Dec 16 2019

ruiu added inline comments to D69665: [llvm-ar] Fix llvm-ar response file reading on Windows.
Dec 16 2019, 10:44 PM · Restricted Project
ruiu added a comment to D71437: [PDB] Print the most redundant type record indices with /summary.

Yeah, I guess that showing that tip line would help users.

Dec 16 2019, 9:04 PM · Restricted Project
ruiu accepted D71060: [LLD][ELF] Add time-trace to ELF LLD (2/2).

LGTM

Dec 16 2019, 8:54 PM · Restricted Project
ruiu accepted D71548: Fix time trace multi threaded support with LLVM_ENABLE_THREADS=OFF.

LGTM

Dec 16 2019, 8:18 PM · Restricted Project
ruiu added a comment to D71509: [ELF][PPC64] Implement IPLT code sequence for non-preemptible IFUNC.

Looks good but I want someone from PPC side to take a look at this change.

Dec 16 2019, 8:18 PM · Restricted Project

Dec 15 2019

ruiu accepted D71518: [ELF] Delete relOff from TargetInfo::writePLT.

LGTM

Dec 15 2019, 10:18 PM · Restricted Project
ruiu added inline comments to D71520: [ELF] Rename .plt to .iplt and decrease EM_PPC{,64} alignment of .glink to 4.
Dec 15 2019, 9:42 PM · Restricted Project
ruiu accepted D71519: [ELF] Add IpltSection.

LGTM

Dec 15 2019, 9:42 PM · Restricted Project

Dec 13 2019

ruiu added a comment to D71161: [ELF] Update st_size when merging a common symbol with a shared symbol.

This behavior seems to make sense but is very subtle. Can you add a comment to explain what this is doing?

Dec 13 2019, 12:35 AM · Restricted Project

Dec 12 2019

ruiu committed rG69da7e29dea6: Revert an accidental commit af5ca40b47b3e85c3add81ccdc0b787c4bc355ae (authored by ruiu).
Revert an accidental commit af5ca40b47b3e85c3add81ccdc0b787c4bc355ae
Dec 12 2019, 10:19 PM
ruiu committed rG6faf8bdcc469: Update the man page (authored by ruiu).
Update the man page
Dec 12 2019, 10:19 PM
ruiu committed rGaf5ca40b47b3: temporary (authored by ruiu).
temporary
Dec 12 2019, 10:19 PM
ruiu closed D71385: Update the man page.
Dec 12 2019, 10:19 PM · Restricted Project
ruiu added a comment to D71437: [PDB] Print the most redundant type record indices with /summary.

Looks like this is an interesting feature, but the output might be a bit too spartan? It feels to me that it is not easy to make this feature's output into an actionable item unless you are not familiar with the debug record format. I wonder if there's an way to print out some "recommendation" to the user, say "<some class> is repeated xxx times. consider moving it out of a header file if it is written in a header.".

Dec 12 2019, 10:10 PM · Restricted Project
ruiu added inline comments to D71060: [LLD][ELF] Add time-trace to ELF LLD (2/2).
Dec 12 2019, 9:43 PM · Restricted Project
ruiu added a comment to D71385: Update the man page.

A question: do we need to specify the particular compression level in docs? My feeling is that
belongs to internal implementation, it is not really useful for user to know the level probably?

Imagine if we switch to another zlib implementation, like was suggested in the original PR,
what if it doesn't have these levels (I do not know if it has).

What would be probably better is to say something like:
"By default we use a compression, that is fast and effective enough, but if you want to
reduce the size of debug sections even more, you can specify -O2 to enable more aggressive,
but slower compression"

Dec 12 2019, 12:34 AM · Restricted Project
ruiu added a comment to D66426: [lld] Enable a watermark of loadable sections to be generated and placed in a note section.

Sorry for asking too many questions, but how do you verify that a watermark matches the contents? Are you creating a new command?

Dec 12 2019, 12:25 AM · Restricted Project
ruiu accepted D70659: [ELF] Allow getErrPlace() to work before Out::bufferStart is set.

LGTM

Dec 12 2019, 12:25 AM · Restricted Project

Dec 11 2019

ruiu added a comment to D70048: [LLD] Add NetBSD support as a new flavor of LLD (nb.lld).

lld is not that Linux-centric as you implied in your comment. We have code for FreeBSD, OpenBSD, AMDGPU and Fuchsia (which isn't even a POSIX system), and adding code for NetBSD is OK and is appreciated. However, the thing we've been trying to avoid is to hard-code a platform-specific knowledge such as library paths to the linker. Combined with the fact that lld doesn't provide a way to enable/disable targets at build-time, all ld.lld executables (sometimes installed as /usr/bin/ld) work the same way no matter where they are run on. That helps those who do cross compilation a lot. That is a very appreciated property of our linker, and I don't think I want make an exception for NetBSD. Even on NetBSD, lld should behave the same as running on other kind of hosts.

Dec 11 2019, 11:57 PM · Restricted Project, Restricted Project, lld
ruiu accepted D71388: [ELF] Add a comment to handleSectionGroup(). NFC.

LGTM

Dec 11 2019, 8:58 PM · Restricted Project
ruiu added a comment to D70919: [Hexagon] Avoid passing unsupported options to lld when -fuse-ld=lld is used.

Remove quotes around check-not.

-fuse-ld=lld is the correct usage. -fuse-ld=ld.lld results in an error message:
error: invalid linker name in argument '-fuse-ld=ld.lld'

Dec 11 2019, 8:46 PM · Restricted Project, Restricted Project
ruiu added inline comments to D71157: [ELF] Refine section group --gc-sections rules to not discard .debug_types.
Dec 11 2019, 8:22 PM · Restricted Project
ruiu updated the diff for D71385: Update the man page.
  • address review comment
Dec 11 2019, 7:52 PM · Restricted Project
ruiu accepted D71059: [LLD][ELF] Add time-trace to ELF LLD (1/2).

LGTM

Dec 11 2019, 7:34 PM · Restricted Project
ruiu added a comment to D70606: LLD: CET shadow stack support on Windows.

Could you tell me about how you generated has_cet.exe? I wonder if there's a way to create an executable in the test instead of submitting a binary file to the repository.

It is a dummy executable (along the lines of int main() {return 0;}) built with Visual Studio 2019. This was inspired by the existing llvm-readobj PDB test which checks has_pdb.exe.

On this topic, yaml2obj has COFF support, although I don't know if that's sufficient for your use case here.

yaml2obj does not yet have CET flag support. What is the preferred path in this situation? We can add it, but it would be only used in this test.

Dec 11 2019, 7:15 PM · Restricted Project, lld
ruiu created D71385: Update the man page.
Dec 11 2019, 7:10 PM · Restricted Project
ruiu added a comment to D71060: [LLD][ELF] Add time-trace to ELF LLD (2/2).

Here are some examples of the output from LLD. One for a llvm-tblgen ThinLTO link (-time-trace-granularity=50000) and a clang Release build (-time-trace-granularity=500). The clang link has some significant gaps in it. I'll see if I can identify where they're from and trace them.

Dec 11 2019, 6:20 PM · Restricted Project
ruiu added inline comments to D71060: [LLD][ELF] Add time-trace to ELF LLD (2/2).
Dec 11 2019, 6:16 PM · Restricted Project
Herald added a reviewer for D70606: LLD: CET shadow stack support on Windows: jhenderson.

Could you tell me about how you generated has_cet.exe? I wonder if there's a way to create an executable in the test instead of submitting a binary file to the repository.

Dec 11 2019, 12:31 AM · Restricted Project, lld
ruiu added inline comments to D71101: [lld][RISCV] Use an e_flags of 0 if there are only binary input files..
Dec 11 2019, 12:22 AM · Restricted Project

Dec 10 2019

ruiu added inline comments to D70447: [Support] ThreadPoolExecutor fixes for Windows/MinGW.
Dec 10 2019, 11:10 PM · Restricted Project
ruiu added a comment to D66426: [lld] Enable a watermark of loadable sections to be generated and placed in a note section.

As I understand it, the scenario is:

  1. Do link; 2) Run the watermark tool to append .note.llvm.watermark; 3) Release SDK; 4) Downstream vendors modify .data and ship to end users; 5) End users verify that .note.llvm.watermark does not match computed watermark of loadable contents.

    The build process before 3) are all controlled. The process should ensure there is no modification to .data between 1) and 2). How do you guarantee a linker side feature can prevent modification? How can you prevent the following:
  2. Do link and generate .note.llvm.watermark in one step 1.5) Modify .data 2) Run the watermark tool to update .note.llvm.watermark

This is a motivation to not have an external tool on the watermarking. That being said, as @chrisjackson has said on more than one occasion, this isn't intended to be a security feature so we are not attempting to detect a malicious attacker. It could be possible for downstream LLD producers to add a local salt to the watermark to make it more secure, should they so choose, I suppose.

Dec 10 2019, 10:34 PM · Restricted Project
ruiu accepted D71069: [ELF][Hexagon]Add TPREL relocation support to Hexagon.

LGTM

Dec 10 2019, 10:22 PM · Restricted Project
ruiu accepted D71326: [ELF] Move a computeIsPreemptible() pass into ICF. NFC.

LGTM

Dec 10 2019, 10:13 PM · Restricted Project
ruiu added a comment to D71059: [LLD][ELF] Add time-trace to ELF LLD (1/2).

Can you share an example output of lld with this patch?

Dec 10 2019, 9:47 PM · Restricted Project
ruiu added inline comments to D71157: [ELF] Refine section group --gc-sections rules to not discard .debug_types.
Dec 10 2019, 9:29 PM · Restricted Project
ruiu added inline comments to D71163: [ELF] --icf: do not fold preemptible symbols.
Dec 10 2019, 9:28 PM · Restricted Project
ruiu accepted D71281: [LLD][ELF][AArch64][ARM] When errata patching, round thunk size to page boundary.

Thank you for fixing the issue. This is an interesting edge case that I haven't thought about that before.

Dec 10 2019, 9:19 PM · Restricted Project
ruiu added inline comments to D71060: [LLD][ELF] Add time-trace to ELF LLD (2/2).
Dec 10 2019, 8:52 PM · Restricted Project
ruiu added a comment to D59780: Support Intel Control-flow Enforcement Technology.

Hmm, I'm sorry but I'm confused. IIRC I had a discussion in the LLVM dev meeting that we were going to submit a change with a single PLT scheme rather than IPLT, so when I said that I'm going to submit a patch, I meant that I'm going to submit a patch for the 1PLT scheme rather than the 2PLT scheme. But this is for the 2PLT scheme. This is not something I want.

Dec 10 2019, 8:43 PM · Restricted Project
ruiu added a comment to D71242: [LLD][ELF]{ARM][AArch64] Add missing classof to patch sections..

classof is an LLVM's way of implementing a dynamic casting, and the code is idiomatic -- we almost always dispatch only by an enum field. So this could be a bit confusing to those who are expecting that idiomatic mechanism? I wonder if it is better to define a new function instead of overloading classof.

We already have such a code pattern

class EhFrameSection final : public SyntheticSection {
public:
  EhFrameSection();
  void writeTo(uint8_t *buf) override;
  void finalizeContents() override;
  bool isNeeded() const override { return !sections.empty(); }
  size_t getSize() const override { return size; }

  static bool classof(const SectionBase *d) {
    return SyntheticSection::classof(d) && d->name == ".eh_frame";
  }
Dec 10 2019, 6:09 PM · Restricted Project
ruiu added a comment to D71242: [LLD][ELF]{ARM][AArch64] Add missing classof to patch sections..

classof is an LLVM's way of implementing a dynamic casting, and the code is idiomatic -- we almost always dispatch only by an enum field. So this could be a bit confusing to those who are expecting that idiomatic mechanism? I wonder if it is better to define a new function instead of overloading classof.

Dec 10 2019, 5:51 PM · Restricted Project

Nov 27 2019

ruiu added a comment to D70447: [Support] ThreadPoolExecutor fixes for Windows/MinGW.

Aren't Parallel and ThreadPool different? We are using parallel_for_loop but we are not using any thread pool.

Nov 27 2019, 11:23 PM · Restricted Project
ruiu committed rGa7acba29c19a: Use InitLLVM in clang-tidy (authored by ruiu).
Use InitLLVM in clang-tidy
Nov 27 2019, 9:02 PM
ruiu closed D70694: Use InitLLVM in clang-tidy.
Nov 27 2019, 9:02 PM · Restricted Project, Restricted Project
ruiu added inline comments to D70606: LLD: CET shadow stack support on Windows.
Nov 27 2019, 7:41 PM · Restricted Project, lld
ruiu added a comment to D70694: Use InitLLVM in clang-tidy.

Sure, let me submit this patch for you.

Nov 27 2019, 6:19 PM · Restricted Project, Restricted Project

Nov 26 2019

ruiu created D70766: Consolidate global variables to a single place.
Nov 26 2019, 11:19 PM · Restricted Project
ruiu added a comment to D70505: [ELF] Replace SymbolTable::forEachSymbol with iterator_range symbols().

I still feel that I prefer the previous code, but that's not a strong preference, so I'm fine with this.

Nov 26 2019, 9:19 PM · Restricted Project
ruiu accepted D70717: [llvm-readobj] - Always print "Predecessors" for version definition sections..

LGTM

Nov 26 2019, 9:00 PM · Restricted Project
ruiu accepted D70606: LLD: CET shadow stack support on Windows.

LGTM with these fixes.

Nov 26 2019, 8:23 PM · Restricted Project, lld
ruiu added a comment to D70694: Use InitLLVM in clang-tidy.

Nope, if you have a commit access, can you commit this?

Nov 26 2019, 6:16 PM · Restricted Project, Restricted Project
ruiu added inline comments to D70658: [LLD][ELF] - Make compression level be dependent on -On..
Nov 26 2019, 12:25 AM · Restricted Project
ruiu accepted D70658: [LLD][ELF] - Make compression level be dependent on -On..

LGTM

Nov 26 2019, 12:06 AM · Restricted Project

Nov 25 2019

ruiu accepted D70695: [ELF] Support input section description .rel[a].dyn in /DISCARD/.

LGTM

Nov 25 2019, 9:06 PM · Restricted Project
ruiu added inline comments to D70606: LLD: CET shadow stack support on Windows.
Nov 25 2019, 6:36 PM · Restricted Project, lld
ruiu committed rG3f76260dc067: Use InitLLVM to setup a pretty stack printer (authored by ruiu).
Use InitLLVM to setup a pretty stack printer
Nov 25 2019, 5:59 PM
ruiu closed D70702: Use InitLLVM to setup a pretty stack printer.
Nov 25 2019, 5:59 PM · Restricted Project, Restricted Project
ruiu created D70702: Use InitLLVM to setup a pretty stack printer.
Nov 25 2019, 5:31 PM · Restricted Project, Restricted Project
ruiu added inline comments to D70659: [ELF] Allow getErrPlace() to work before Out::bufferStart is set.
Nov 25 2019, 4:57 PM · Restricted Project
ruiu accepted D70694: Use InitLLVM in clang-tidy.

LGTM

Nov 25 2019, 4:55 PM · Restricted Project, Restricted Project

Nov 24 2019

ruiu added inline comments to D70606: LLD: CET shadow stack support on Windows.
Nov 24 2019, 4:59 PM · Restricted Project, lld

Nov 21 2019

ruiu added a comment to D70507: Fix PR44093: use of stale error stream when linking.

James Knight has submitted a very similar change to fix the problem.

Nov 21 2019, 8:33 PM · Restricted Project, lld
ruiu added a comment to D70557: [lld][COFF] Add support for /map.

Thank you for the patch. Overall, I think it is a good thing to add link.exe-compatible /map file output to lld.

Nov 21 2019, 8:33 PM · Restricted Project, lld

Nov 20 2019

ruiu accepted D70492: LLD: Assign the stderrOS and stdoutOS globals to the new argument values, before using via enableColors..

LGTM

Nov 20 2019, 8:23 PM · Restricted Project
ruiu added a comment to D70505: [ELF] Replace SymbolTable::forEachSymbol with iterator_range symbols().

This is my personal taste, but I feel like the former code is simpler, though it doesn't look elegant. The new code looks a bit too smart to me. I also have a little concern to define start and end to SymbolTable class because it is not very clear what are iterated. SymbolTable contains not only symbols but other members.

Would it alleviate your concerns if it were more like:

for (Symbol *sym : symtab->symbols())

?

Nov 20 2019, 7:29 PM · Restricted Project
ruiu added a comment to D70505: [ELF] Replace SymbolTable::forEachSymbol with iterator_range symbols().

This is my personal taste, but I feel like the former code is simpler, though it doesn't look elegant. The new code looks a bit too smart to me. I also have a little concern to define start and end to SymbolTable class because it is not very clear what are iterated. SymbolTable contains not only symbols but other members.

Nov 20 2019, 5:58 PM · Restricted Project
ruiu added inline comments to D70507: Fix PR44093: use of stale error stream when linking.
Nov 20 2019, 3:58 PM · Restricted Project, lld

Nov 19 2019

ruiu added a comment to D68062: Propeller lld framework for basicblock sections.

Since you have moved all the files to a separate directory, it is probably better to create README.md to explain what Propeller is and how it works. You may want to move some file comments to that file.

Nov 19 2019, 10:48 PM · Restricted Project
ruiu accepted D70468: [ELF] Error if -Ttext-segment is specified.

LGTM

Nov 19 2019, 9:32 PM · Restricted Project
ruiu added a comment to D70421: Initialize global vectors with invalid members to catch uninitialization errors.

Does it make sense to accumulate all global variables in the one place? Like place them to a separate file with initGlobals method?
I think one of their problems is that they spreaded everywhere and initialized from many places.

Nov 19 2019, 9:04 PM · Restricted Project
ruiu added a comment to D66426: [lld] Enable a watermark of loadable sections to be generated and placed in a note section.

I think this feature is worth more attention. There may be someone who wants to start using this once it's landed, and I'd make sure that we satisfy their needs. Do you mind if I ask you to start a thread on llvm-dev to propose this feature? I think that even a comment like "we'll use this" is valuable. tlike "

Nov 19 2019, 8:57 PM · Restricted Project
ruiu added inline comments to D70420: Use lld::make<T> to make TpiSource objects.
Nov 19 2019, 8:25 PM · Restricted Project
ruiu committed rG47feae5dd61d: Use lld::make<T> to make TpiSource objects (authored by ruiu).
Use lld::make<T> to make TpiSource objects
Nov 19 2019, 8:21 PM
ruiu closed D70420: Use lld::make<T> to make TpiSource objects.
Nov 19 2019, 8:21 PM · Restricted Project
ruiu added a comment to D70398: [lld] Support copy relocations for TLS symbols.

I think I agree with dalias. I don't get a point to allow users to use copy relocations liberally for a new ISA/ABI like RISC-V, and it looks like it is a problem that we should fix compilers as soon as possible, rather than adopting the linker to what compilers emit at the moment. Even though I can see the reason why you wanted to add this to lld, I can also imagine that that would be a regrettable decision in the long term.

Nov 19 2019, 7:57 PM · Restricted Project
ruiu added a comment to D70399: [llvm-readobj/llvm-readelf] - Improve dumping of versioning sections..

I agree that making the field an enumeration is an improvement, but I also feel that the closing bracket ] on a different line is a bit verbose.

Ideally we can make it on one line, e.g. Flags [], Flags [ Info (0x4), 0x8] (0x8 is an unspecified bit field. Bikeshedding on the representation welcomed!)

Nov 19 2019, 6:05 PM · Restricted Project
ruiu added inline comments to D70468: [ELF] Error if -Ttext-segment is specified.
Nov 19 2019, 5:56 PM · Restricted Project
ruiu added a comment to D70421: Initialize global vectors with invalid members to catch uninitialization errors.

The other way of testing this is actually invoking the linker twice from main(), only in a test mode (specified by some environment variable), to see if it works. It is doable, and it should be able to detect all errors, but I'm not sure if we want to run tests twice.

Nov 19 2019, 12:22 AM · Restricted Project
ruiu added inline comments to D70146: [ELF] Improve --gc-sections compatibility with GNU ld regarding section groups.
Nov 19 2019, 12:22 AM · Restricted Project

Nov 18 2019

ruiu added a comment to D70398: [lld] Support copy relocations for TLS symbols.

James, I'm sorry that I think I missed your second comment. So if gcc and clang uses LE model even for extern TLS variables, I agree with you that it's odd. They may have wanted to avoid the cost of more general access models, but because the linker is able to relax them, that shouldn't be a big problem.

Nov 18 2019, 11:35 PM · Restricted Project
ruiu accepted D70146: [ELF] Improve --gc-sections compatibility with GNU ld regarding section groups.

LGTM

Nov 18 2019, 11:12 PM · Restricted Project
ruiu created D70421: Initialize global vectors with invalid members to catch uninitialization errors.
Nov 18 2019, 11:00 PM · Restricted Project
ruiu created D70420: Use lld::make<T> to make TpiSource objects.
Nov 18 2019, 10:20 PM · Restricted Project
ruiu added a comment to D70378: [LLD][COFF] Fix missing cache cleanup in COFF::link().

I share your concern and I'm sorry about leaving so many global variables as uninitialized in the second run of the linker in the same process. But I'd like to find and fix the problems without abolishing global variables in lld, as I don't actually dislike them. One thing we could do is initializing global variables with invalid data -- like -1 or {nullptr} so that you'd get an error even on the first run if you do not reset them before use. I'll create a patch and send it to you.

Nov 18 2019, 9:40 PM · lld, Restricted Project
ruiu added a reverting change for rG17e37ba57a69: Fix shared lib build.: rG45f8ee5f3c5a: Revert "Fix shared lib build.".
Nov 18 2019, 7:11 PM
ruiu committed rG45f8ee5f3c5a: Revert "Fix shared lib build." (authored by ruiu).
Revert "Fix shared lib build."
Nov 18 2019, 7:11 PM
ruiu added a comment to D70398: [lld] Support copy relocations for TLS symbols.

Copy relocation is a hack to allow non-PIC object files to be linked with DSOs. And it is a bit dangerous hack because people don't usually understand the exact semantics of the copy relocation and can make an ABI-breaking change without knowing it. I described the problem here: https://github.com/llvm/llvm-project/blob/master/lld/ELF/Relocations.cpp#L517-L558

Nov 18 2019, 6:53 PM · Restricted Project