Page MenuHomePhabricator

seiya (Seiya Nuta)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 30 2019, 8:08 PM (42 w, 1 d)

Recent Activity

Dec 20 2019

seiya added inline comments to D71647: [llvm-objcopy][MachO] Handle relocation entries where r_extern is 0.
Dec 20 2019, 2:11 AM · Restricted Project
seiya added a comment to D71647: [llvm-objcopy][MachO] Handle relocation entries where r_extern is 0.

BTW, I noticed that we need a same fix for n_sect field in nlist (in a separate patch).

Dec 20 2019, 2:02 AM · Restricted Project

Dec 18 2019

seiya added inline comments to D71647: [llvm-objcopy][MachO] Handle relocation entries where r_extern is 0.
Dec 18 2019, 5:21 PM · Restricted Project

Dec 17 2019

seiya updated the diff for D71647: [llvm-objcopy][MachO] Handle relocation entries where r_extern is 0.

Update a comment in test. NFC.

Dec 17 2019, 9:53 PM · Restricted Project
seiya created D71647: [llvm-objcopy][MachO] Handle relocation entries where r_extern is 0.
Dec 17 2019, 9:53 PM · Restricted Project

Dec 15 2019

seiya committed rG9e119ad69df7: [llvm-objcopy][MachO] Implement --add-section (authored by seiya).
[llvm-objcopy][MachO] Implement --add-section
Dec 15 2019, 9:15 PM
seiya closed D66283: [llvm-objcopy][MachO] Implement --add-section.
Dec 15 2019, 9:15 PM · Restricted Project

Dec 12 2019

seiya abandoned D71331: [llvm-objcopy][MachO][NFC] Allow section to own its contents.

Merged into D66283: I think it does not have to be a separate patch because using StringSaver simplified what I wanted to do in this patch.

Dec 12 2019, 9:16 PM · Restricted Project
seiya updated the diff for D66283: [llvm-objcopy][MachO] Implement --add-section.
Dec 12 2019, 9:08 PM · Restricted Project
seiya added inline comments to D71331: [llvm-objcopy][MachO][NFC] Allow section to own its contents.
Dec 12 2019, 6:06 PM · Restricted Project

Dec 11 2019

seiya added a comment to D63309: [llvm-objcopy][MachO] Rebuild the symbol/string table in the writer.

From https://developer.apple.com/documentation/kernel/relocation_info?changes=latest_major&language=ob_7:

r_symbolnum

Indicates either an index into the symbol table (when the r_extern field is set to 1) or a section number (when the r_extern field is set to 0). As previously mentioned, sections are ordered from 1 to 255 in the order in which they appear in the LC_SEGMENT load commands. This field is set to R_ABS for relocation entries for absolute symbols, which need no relocation.

The current Mach-O reader and writer do not handle r_extern=0 relocations.

// MachO/MachOReader
void MachOReader::setSymbolInRelocationInfo(Object &O) const {
  for (auto &LC : O.LoadCommands)
    for (auto &Sec : LC.Sections)
      for (auto &Reloc : Sec.Relocations)
        if (!Reloc.Scattered) {
          auto *Info = reinterpret_cast<MachO::relocation_info *>(&Reloc.Info);
          Reloc.Symbol = O.SymTable.getSymbolByIndex(Info->r_symbolnum);  //// this can reference a section when r_extern=0
        }
}
Dec 11 2019, 11:58 PM · Restricted Project
seiya added a comment to D66283: [llvm-objcopy][MachO] Implement --add-section.

There seems to be some stuff to do with 64-bit versus normal segments in here, which I don't fully follow due to it being Mach-O, but your test only seems to handle the 64-bit segment type. Should it be extended?

Dec 11 2019, 9:32 PM · Restricted Project
seiya updated the diff for D66283: [llvm-objcopy][MachO] Implement --add-section.

Added 32-bit object support.

Dec 11 2019, 9:32 PM · Restricted Project
seiya updated the diff for D71331: [llvm-objcopy][MachO][NFC] Allow section to own its contents.

Addressed review comments.

Dec 11 2019, 9:25 PM · Restricted Project
seiya updated the diff for D66283: [llvm-objcopy][MachO] Implement --add-section.

Added a newline at EOF.

Dec 11 2019, 2:07 AM · Restricted Project
seiya added inline comments to D66283: [llvm-objcopy][MachO] Implement --add-section.
Dec 11 2019, 2:04 AM · Restricted Project
seiya updated the diff for D66283: [llvm-objcopy][MachO] Implement --add-section.

Addressed review comments and isolate some changes into D71331.

Dec 11 2019, 2:03 AM · Restricted Project
seiya created D71331: [llvm-objcopy][MachO][NFC] Allow section to own its contents.
Dec 11 2019, 2:02 AM · Restricted Project

Nov 24 2019

seiya committed rGd72a8a4dd5ba: [llvm-objcopy][MachO] Implement --dump-section (authored by seiya).
[llvm-objcopy][MachO] Implement --dump-section
Nov 24 2019, 7:48 PM
seiya closed D66408: [llvm-objcopy][MachO] Implement --dump-section.
Nov 24 2019, 7:47 PM · Restricted Project
seiya updated the diff for D66408: [llvm-objcopy][MachO] Implement --dump-section.

Addressed a comment.

Nov 24 2019, 7:25 PM · Restricted Project

Nov 22 2019

seiya added a comment to D66408: [llvm-objcopy][MachO] Implement --dump-section.

I'll commit this patch on next Monday.

Nov 22 2019, 8:20 PM · Restricted Project

Nov 21 2019

seiya updated the diff for D66408: [llvm-objcopy][MachO] Implement --dump-section.

Rebased. NFCI.

Nov 21 2019, 7:57 PM · Restricted Project
seiya accepted D70475: [llvm-objcopy][MachO] Fix symbol order in the symbol table.

LGTM too. Thanks for the fix!

Nov 21 2019, 1:07 AM · Restricted Project

Nov 15 2019

seiya accepted D70212: [llvm-objcopy][MachO] Implement --redefine-sym and --redefine-syms.
Nov 15 2019, 7:51 AM · Restricted Project

Nov 14 2019

seiya committed rGbc11830c6a67: [llvm-objcopy][MachO] Implement --remove-section (authored by seiya).
[llvm-objcopy][MachO] Implement --remove-section
Nov 14 2019, 9:28 PM
seiya closed D66282: [llvm-objcopy][MachO] Implement --remove-section.
Nov 14 2019, 9:28 PM · Restricted Project

Oct 30 2019

seiya committed rG9bbf2a15442e: [llvm-objcopy][MachO] Implement --strip-all (authored by seiya).
[llvm-objcopy][MachO] Implement --strip-all
Oct 30 2019, 10:29 PM
seiya closed D66281: [llvm-objcopy][MachO] Implement --strip-all.
Oct 30 2019, 10:29 PM · Restricted Project
seiya committed rGd6b72b0e4df0: [llvm-objcopy] Add REQUIRES directive to fix a test (authored by seiya).
[llvm-objcopy] Add REQUIRES directive to fix a test
Oct 30 2019, 12:21 AM

Oct 29 2019

seiya committed rG1e589f67ef72: [llvm-objcopy][MachO] Support indirect symbol table (authored by seiya).
[llvm-objcopy][MachO] Support indirect symbol table
Oct 29 2019, 11:26 PM
seiya closed D66280: [llvm-objcopy][MachO] Support indirect symbol table.
Oct 29 2019, 11:25 PM · Restricted Project

Oct 28 2019

seiya committed rG7f19dd1ebff0: [llvm-objcopy][MachO] Implement --only-section (authored by seiya).
[llvm-objcopy][MachO] Implement --only-section
Oct 28 2019, 12:01 AM
seiya closed D65541: [llvm-objcopy][MachO] Implement --only-section.
Oct 28 2019, 12:01 AM · Restricted Project

Oct 25 2019

seiya accepted D69419: [llvm-objcopy][MachO] Add support for min os version load commands.

LGTM too.

Oct 25 2019, 4:02 AM · Restricted Project
seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Updated a test.

Oct 25 2019, 4:01 AM · Restricted Project
seiya added a comment to D65541: [llvm-objcopy][MachO] Implement --only-section.

Ping @alexshap (and also @jhenderson and @rupprecht would want to confirm before landing). After this patch got accepted, in order to merge it, I added wildcard support and a test in this diff.

Oct 25 2019, 2:46 AM · Restricted Project

Oct 17 2019

seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Before committing this patch, I noticed that we need to consider how llvm-objcopy should handle wildcards in Mach-O support.

Oct 17 2019, 6:03 PM · Restricted Project

Oct 16 2019

seiya added a comment to D65541: [llvm-objcopy][MachO] Implement --only-section.

Thank you for your review!

Oct 16 2019, 6:55 PM · Restricted Project
seiya added a comment to D65541: [llvm-objcopy][MachO] Implement --only-section.

@alexshap Could you review this patch? It's accepted but I'd like someone who's familiar with Mach-O to confirm (a ping in case you missed changes in this patch).

Oct 16 2019, 1:45 AM · Restricted Project

Oct 15 2019

seiya committed rL374890: Request commit access for seiya.
Request commit access for seiya
Oct 15 2019, 7:10 AM

Oct 9 2019

seiya added inline comments to D66282: [llvm-objcopy][MachO] Implement --remove-section.
Oct 9 2019, 12:20 AM · Restricted Project

Oct 7 2019

seiya added a comment to D65541: [llvm-objcopy][MachO] Implement --only-section.

Friendly ping: I'd like someone who is familiar with Mach-O to review this patch.

Oct 7 2019, 8:31 PM · Restricted Project
seiya updated the diff for D66282: [llvm-objcopy][MachO] Implement --remove-section.

Rebased.

Oct 7 2019, 8:30 PM · Restricted Project
seiya updated the diff for D66281: [llvm-objcopy][MachO] Implement --strip-all.

Rebased. No changes intended.

Oct 7 2019, 8:27 PM · Restricted Project

Oct 3 2019

seiya updated the diff for D66280: [llvm-objcopy][MachO] Support indirect symbol table.
  • Added a comment.
Oct 3 2019, 5:20 PM · Restricted Project

Oct 1 2019

seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Addressed a review comment.

Oct 1 2019, 7:28 AM · Restricted Project

Sep 30 2019

seiya added inline comments to D65541: [llvm-objcopy][MachO] Implement --only-section.
Sep 30 2019, 8:18 PM · Restricted Project
seiya updated the diff for D66280: [llvm-objcopy][MachO] Support indirect symbol table.

clang-formatted

Sep 30 2019, 8:14 PM · Restricted Project
seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Addressed review comments.

Sep 30 2019, 8:13 PM · Restricted Project

Sep 29 2019

seiya updated the diff for D66280: [llvm-objcopy][MachO] Support indirect symbol table.

Addressed a review comment.

Sep 29 2019, 8:13 PM · Restricted Project
seiya updated the diff for D65541: [llvm-objcopy][MachO] Implement --only-section.

Add MachOConfig.cpp to isolate command-line validation logic from MachOObjCopy.cpp.

Sep 29 2019, 8:11 PM · Restricted Project

Sep 24 2019

seiya committed rGc83eefcfda7c: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC. (authored by seiya).
[llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC.
Sep 24 2019, 2:41 AM
seiya committed rL372712: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
[llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC.
Sep 24 2019, 2:37 AM
seiya closed D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
Sep 24 2019, 2:37 AM · Restricted Project

Sep 20 2019

seiya added a comment to D67689: [llvm-objcopy] Add support for --gap-fill and --pad-to options.

Commented regarding tests and nits. Could you update docs/CommandGuide/llvm-objcopy.rst as well?

Sep 20 2019, 12:04 AM · Restricted Project

Sep 19 2019

seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
  • Addressed a review comment.
Sep 19 2019, 11:23 PM · Restricted Project

Sep 18 2019

seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
  • Added a comment for parseELFConfig.
Sep 18 2019, 2:54 AM · Restricted Project
seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
Sep 18 2019, 2:49 AM · Restricted Project

Sep 17 2019

seiya added a comment to D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

Thanks you for feedbacks. Putting it all together, the @alexshap's suggestion (D67139#inline-607235) looks desirable to me because:

Sep 17 2019, 2:49 AM · Restricted Project

Sep 15 2019

seiya added a comment to D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

What about having ELFcopyConfig inherit from CopyConfig?

How about this design? I believe moving the CopyConfig is not a big cost.

Sep 15 2019, 6:07 PM · Restricted Project

Sep 13 2019

seiya added inline comments to D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
Sep 13 2019, 4:04 AM · Restricted Project
seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

Pass only ELFCopyConfig instead of passing both CopyConfig and ELFCopyConfig.

Sep 13 2019, 2:24 AM · Restricted Project

Sep 12 2019

seiya updated the diff for D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

Adopt @jhenderson's idea D67139#1657802.

Sep 12 2019, 3:15 AM · Restricted Project

Sep 10 2019

seiya added inline comments to D67060: [Support] Support restoring colors in WithColor.
Sep 10 2019, 2:28 AM · Restricted Project
seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.
  • Removed an unrelated change from the diff.
Sep 10 2019, 2:28 AM · Restricted Project
seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

This patch looks good, but I'd add a test for the new code. Is there any place in LLVM where the current color is saved and restored? If there's such code, you can rewrite the code to see if your code would work as expected.

Sep 10 2019, 2:24 AM · Restricted Project
seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.

Addressed review comments

Sep 10 2019, 2:13 AM · Restricted Project

Sep 9 2019

seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.

Addressed review comments.

Sep 9 2019, 7:11 PM · Restricted Project
seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.
  • Abandon WithColorContext: save the previous color settings in WithColor instead.
  • Update the description.
Sep 9 2019, 7:24 AM · Restricted Project

Sep 5 2019

seiya added a comment to D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..

I've got another suggestion: have separate COFFConfig/ELFConfig etc classes that only contain the interpreted versions and lazily construct them prior to calling the corresponding code path. CopyConfig would contain the raw unparsed strings (perhaps rename them to "RawSymbolsToAdd" etc). Perhaps a higher-level Config class could exist just to wrap them and pass them to executeObjcopyOnBinary etc.

Rough outline:

CopyConfig.h:

class CopyConfig {
  std::vector<StringRef> RawSymbolsToAdd;
  ...
};

class ELFConfig {
  std::vector<NewSymbolInfo> SymbolsToAdd;
  ...
};

// etc for COFF, Mach-O...

llvm-objcopy.cpp:

struct Configurations {
  CopyConfig Common;
  Optional<ELFConfig> ELF;
  ...
}

...

static Error executeObjcopyOnBinary(Configurations &Cfgs, ...) {
  if (/*isElf*/...) {
    if (!Cfgs.ELF)
     Cfgs.ELF.emplace(Cfgs.Common);
    return elf::executeObjcopyOnBinary(Cfgs.Common, Cfgs.ELF, ...)
  }
  ...
}

LGTM. I'll implement the idea.

Sep 5 2019, 8:03 PM · Restricted Project

Sep 4 2019

seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

I don't think there's a way to get the current color from the terminal, so you should modify changeColor so that the function remembers the color given to the function.

Sep 4 2019, 2:37 AM · Restricted Project

Sep 3 2019

seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

This looks better, but I don't think we need the concept of context. If a function takes a context object, it is usually expected that you can interlace function calls with different context objects freely. Say, you have context object A and B, then you literally have two contexts, and they don't interfere with each other. However, that's not the case for the terminal color. The current color of text output on console is naturally and fundamentally a global state associated to each terminal, and therefore there is only one context. So, I don't think that allowing multiple contexts captures the real concept correctly.

I believe the following scheme models captures the reality better:

  1. Add a new method, getColor, to raw_fd_stream, which returns the current color set by changeColor before
  2. Make a change to WithColor so that it remembers the current color (obtained by calling getColor) before changing color
  3. In the destructor, WithColor restores the previous color
Sep 3 2019, 11:57 PM · Restricted Project
seiya created D67139: [llvm-objcopy] Refactor ELF-specific config out to ELFCopyConfig. NFC..
Sep 3 2019, 7:24 PM · Restricted Project
seiya added a comment to D66281: [llvm-objcopy][MachO] Implement --strip-all.

@seyia, sorry about the delays with code review, does this particular diff have any unreviewed dependencies ?

Sep 3 2019, 7:04 PM · Restricted Project
seiya added inline comments to D67060: [Support] Support restoring colors in WithColor.
Sep 3 2019, 6:52 PM · Restricted Project
seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

Did you find any existing usage of WithColor that depends on the behavior of resetting the color? If not, you don't have to care too much about keeping the compatibility, as that is not really an intended feature as far as I understand. You probably can just change the behavior of WithColor so that it restores to the previous color instead of resseting.

Sep 3 2019, 6:34 PM · Restricted Project
seiya updated the diff for D67060: [Support] Support restoring colors in WithColor.

Moved NestableWithColor features into WithColor.

Sep 3 2019, 6:27 PM · Restricted Project
seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

Instead of adding a new class, do you think you can change the behavior of WithColor so that it restores the original colors instead of resetting it?

This would be my preference too, if it's viable. I don't think WithColor has been around that long, so I suspect it may be straightforward to identify existing usage patterns, and I'd be surprised if any expect restoration to default.

Sep 3 2019, 12:55 AM · Restricted Project

Sep 2 2019

seiya added reviewers for D67060: [Support] Support restoring colors in WithColor: jhenderson, rupprecht, ruiu, MaskRay.

Added reviewers who may be familiar with this part.

Sep 2 2019, 12:48 AM · Restricted Project
seiya updated the summary of D67060: [Support] Support restoring colors in WithColor.
Sep 2 2019, 12:44 AM · Restricted Project
seiya updated the summary of D67060: [Support] Support restoring colors in WithColor.
Sep 2 2019, 12:44 AM · Restricted Project
seiya added a comment to D67060: [Support] Support restoring colors in WithColor.

This patch does not include tests but it will be used in D65191.

Sep 2 2019, 12:27 AM · Restricted Project
seiya created D67060: [Support] Support restoring colors in WithColor.
Sep 2 2019, 12:24 AM · Restricted Project

Aug 27 2019

seiya added a comment to D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing.

Test cases for the new code?

I forgot to add NFC in the title. This patch does not introduce new features thus we don't need new tests for it I think.

Aug 27 2019, 8:38 PM · Restricted Project
seiya retitled D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing from [llvm-objcopy] Accept MachO formats in commad-line parsing. NFC. to [llvm-objcopy] Accept MachO formats in commad-line parsing.
Aug 27 2019, 8:38 PM · Restricted Project
seiya added a comment to D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing.

Test cases for the new code?

Aug 27 2019, 8:33 PM · Restricted Project
seiya retitled D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing from [llvm-objcopy] Accept MachO formats in commad-line parsing to [llvm-objcopy] Accept MachO formats in commad-line parsing. NFC..
Aug 27 2019, 8:31 PM · Restricted Project

Aug 26 2019

seiya added inline comments to D65191: [llvm-objdump] Implement highlighting.
Aug 26 2019, 7:57 PM · Restricted Project
seiya updated the diff for D66409: [llvm-objcopy][MachO] Implement -Obinary.

Addressed a review comment.

Aug 26 2019, 7:57 PM · Restricted Project
seiya added inline comments to D66409: [llvm-objcopy][MachO] Implement -Obinary.
Aug 26 2019, 7:57 PM · Restricted Project
seiya updated the diff for D66409: [llvm-objcopy][MachO] Implement -Obinary.

Addressed review comments.

Aug 26 2019, 7:57 PM · Restricted Project
seiya added inline comments to D66409: [llvm-objcopy][MachO] Implement -Obinary.
Aug 26 2019, 7:57 PM · Restricted Project
seiya added inline comments to D66517: [llvm-objdump] - Remove one overload of reportError. NFCI..
Aug 26 2019, 3:14 AM · Restricted Project
seiya added inline comments to D66517: [llvm-objdump] - Remove one overload of reportError. NFCI..
Aug 26 2019, 2:33 AM · Restricted Project
seiya planned changes to D65541: [llvm-objcopy][MachO] Implement --only-section.

I'll submit another patch to refactor CopyConfig as described in https://reviews.llvm.org/D65541#inline-594247

Aug 26 2019, 1:36 AM · Restricted Project
seiya added inline comments to D66407: [llvm-objcopy][MachO] Implement -Ibinary.
Aug 26 2019, 1:34 AM · Restricted Project
seiya updated the diff for D66407: [llvm-objcopy][MachO] Implement -Ibinary.

Addressed review comments and added test for arm64.

Aug 26 2019, 1:27 AM · Restricted Project
seiya added inline comments to D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing.
Aug 26 2019, 1:22 AM · Restricted Project
seiya updated the diff for D66449: [llvm-objcopy] Accept MachO formats in commad-line parsing.

Addressed review comments.

Aug 26 2019, 1:22 AM · Restricted Project