Page MenuHomePhabricator

grimar (George Rimar)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 21 2015, 12:36 AM (208 w, 4 d)

Recent Activity

Yesterday

grimar added inline comments to D67761: [ELF] Error if the linked-to section of a SHF_LINK_ORDER section is discarded.
Thu, Sep 19, 6:59 AM · Restricted Project
grimar created D67759: [llvm-readobj] - Simplify stack-sizes.test test case..
Thu, Sep 19, 5:22 AM
grimar added a child revision for D67757: [yaml2obj/obj2yaml] - Add a support for .stack_sizes sections.: D67759: [llvm-readobj] - Simplify stack-sizes.test test case..
Thu, Sep 19, 5:22 AM
grimar updated the summary of D67757: [yaml2obj/obj2yaml] - Add a support for .stack_sizes sections..
Thu, Sep 19, 4:45 AM
grimar created D67757: [yaml2obj/obj2yaml] - Add a support for .stack_sizes sections..
Thu, Sep 19, 4:45 AM
grimar added a comment to D67754: [ELF] Fix two null pointer dereferences related to SHF_LINK_ORDER with --gc-sections.

Two questions:

  1. Why it is not a error? As far I understood after reading comments for PR, this is what bfd would do?

(https://bugs.llvm.org/show_bug.cgi?id=43147#c8)

  1. Should we instead disable GC for that case maybe?
Thu, Sep 19, 3:46 AM · Restricted Project
grimar accepted D67449: [llvm-ar] Include a line number when failing to parse an MRI script.

LGTM with a nit.

Thu, Sep 19, 1:26 AM · Restricted Project

Wed, Sep 18

grimar accepted D67656: [llvm-objcopy] Add --set-section-alignment.

It looks good to me, but please wait until someone else approve this too.

Wed, Sep 18, 1:48 AM · Restricted Project

Tue, Sep 17

grimar added a comment to D67526: [RISCV][NFC] Use NoRegister instead of 0 literal.

It broke BB: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/19706/steps/build-unified-tree/logs/stdio

Tue, Sep 17, 7:10 AM · Restricted Project
grimar committed rGa3569aced05d: [llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and… (authored by grimar).
[llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and…
Tue, Sep 17, 7:01 AM
grimar committed rL372122: [llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and….
[llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and…
Tue, Sep 17, 6:57 AM
grimar closed D67547: [llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and report warnings about that..
Tue, Sep 17, 6:57 AM · Restricted Project
grimar created D67657: [yaml2obj/obj2yaml] - Do not trigger llvm_unreachable when dumping/parsing relocations and e_machine is unsupported..
Tue, Sep 17, 6:25 AM
grimar added inline comments to D67656: [llvm-objcopy] Add --set-section-alignment.
Tue, Sep 17, 5:50 AM · Restricted Project
grimar committed rG589293800afa: [llvm-readobj] - Test PPC64 relocations properly. (authored by grimar).
[llvm-readobj] - Test PPC64 relocations properly.
Tue, Sep 17, 5:07 AM
grimar committed rL372110: [llvm-readobj] - Test PPC64 relocations properly..
[llvm-readobj] - Test PPC64 relocations properly.
Tue, Sep 17, 5:04 AM
grimar closed D67617: [llvm-readobj] - Test PPC64 relocations properly..
Tue, Sep 17, 5:04 AM · Restricted Project
grimar committed rG82d83733dd70: [obj2yaml] - Support PPC64 relocation types. (authored by grimar).
[obj2yaml] - Support PPC64 relocation types.
Tue, Sep 17, 5:04 AM
grimar committed rL372109: [obj2yaml] - Support PPC64 relocation types..
[obj2yaml] - Support PPC64 relocation types.
Tue, Sep 17, 4:59 AM
grimar closed D67615: [obj2yaml] - Support PPC64 relocation types..
Tue, Sep 17, 4:59 AM · Restricted Project
grimar committed rGcfc0ba3852ca: [yaml2obj/obj2yaml] - Allow setting an arbitrary values for e_machine. (authored by grimar).
[yaml2obj/obj2yaml] - Allow setting an arbitrary values for e_machine.
Tue, Sep 17, 4:53 AM
grimar committed rL372108: [yaml2obj/obj2yaml] - Allow setting an arbitrary values for e_machine..
[yaml2obj/obj2yaml] - Allow setting an arbitrary values for e_machine.
Tue, Sep 17, 4:53 AM
grimar closed D67652: [yaml2obj/obj2yaml] - Allow setting an arbitrary values for e_machine..
Tue, Sep 17, 4:53 AM · Restricted Project
grimar retitled D67652: [yaml2obj/obj2yaml] - Allow setting an arbitrary values for e_machine. from [yaml2obj/obj2yam] - Allow setting an arbitrary values for e_machine. to [yaml2obj/obj2yaml] - Allow setting an arbitrary values for e_machine..
Tue, Sep 17, 4:53 AM · Restricted Project
grimar created D67652: [yaml2obj/obj2yaml] - Allow setting an arbitrary values for e_machine..
Tue, Sep 17, 4:34 AM · Restricted Project
grimar added a comment to D67615: [obj2yaml] - Support PPC64 relocation types..

Add a support for PPC64 relocations.

Add support for or just Support PPC64 relocation types.

I used relocation types because I wanted to emphasize we supported more relocation types:)

Tue, Sep 17, 3:16 AM · Restricted Project
grimar retitled D67615: [obj2yaml] - Support PPC64 relocation types. from [obj2yaml] - Add support for PPC64 relocations. to [obj2yaml] - Support PPC64 relocation types..
Tue, Sep 17, 3:16 AM · Restricted Project
grimar retitled D67615: [obj2yaml] - Support PPC64 relocation types. from [obj2yaml] - Add a support for PPC64 relocations. to [obj2yaml] - Add support for PPC64 relocations..
Tue, Sep 17, 3:02 AM · Restricted Project
grimar committed rG48de660bbf0d: [llvm-readobj] - Fix BB after r372087. (authored by grimar).
[llvm-readobj] - Fix BB after r372087.
Tue, Sep 17, 2:29 AM
grimar committed rL372089: [llvm-readobj] - Fix BB after r372087..
[llvm-readobj] - Fix BB after r372087.
Tue, Sep 17, 2:26 AM
grimar added inline comments to D67615: [obj2yaml] - Support PPC64 relocation types..
Tue, Sep 17, 2:26 AM · Restricted Project
grimar committed rGde1bef0b1b2d: [llvm-readobj] - Fix a TODO in elf-reloc-zero-name-or-value.test. (authored by grimar).
[llvm-readobj] - Fix a TODO in elf-reloc-zero-name-or-value.test.
Tue, Sep 17, 2:11 AM
grimar committed rL372087: [llvm-readobj] - Fix a TODO in elf-reloc-zero-name-or-value.test..
[llvm-readobj] - Fix a TODO in elf-reloc-zero-name-or-value.test.
Tue, Sep 17, 2:10 AM
grimar closed D67609: [llvm-readobj] - Fix a TODO in elf-reloc-zero-name-or-value.test..
Tue, Sep 17, 2:10 AM · Restricted Project
grimar added a comment to D67449: [llvm-ar] Include a line number when failing to parse an MRI script.

I wish we can create a new function for MRI error reporting but as it stands, runMRIScript can call readLibrary, addChildMember, and addMember which can in turn call fail or failIfError, so fail has to be aware of the MRI line number. +@grimar @jhenderson in case they have suggestions.

The only suggestion I have is to pass in an error handler from the calling code that can be used to generate an Error instance with a message customised to the caller. Default would be to do nothing special of course, but it would allow the MRI parser to pass one that refers back to a handler that appends the line number. That's potentially quite a significant refactor, so might be worth delaying to a subsequent change, but it might also be worth being done before this, so that we don't have to introduce the if statement in fail. What do people think?

I am not good familar with this tool's code yet, but at first look introducing a custom handler looks reasonable (that would help to remove most of these new variables it seems).

Tue, Sep 17, 2:07 AM · Restricted Project
grimar added a comment to D67449: [llvm-ar] Include a line number when failing to parse an MRI script.

I wish we can create a new function for MRI error reporting but as it stands, runMRIScript can call readLibrary, addChildMember, and addMember which can in turn call fail or failIfError, so fail has to be aware of the MRI line number. +@grimar @jhenderson in case they have suggestions.

The only suggestion I have is to pass in an error handler from the calling code that can be used to generate an Error instance with a message customised to the caller. Default would be to do nothing special of course, but it would allow the MRI parser to pass one that refers back to a handler that appends the line number. That's potentially quite a significant refactor, so might be worth delaying to a subsequent change, but it might also be worth being done before this, so that we don't have to introduce the if statement in fail. What do people think?

Tue, Sep 17, 2:01 AM · Restricted Project
grimar committed rG505553495c43: [llvm-readobj] - Refactor the code. (authored by grimar).
[llvm-readobj] - Refactor the code.
Tue, Sep 17, 1:57 AM
grimar committed rL372083: [llvm-readobj] - Refactor the code..
[llvm-readobj] - Refactor the code.
Tue, Sep 17, 1:52 AM
grimar closed D67624: [llvm-readobj] - Refactor the code..
Tue, Sep 17, 1:52 AM · Restricted Project
grimar retitled D67624: [llvm-readobj] - Refactor the code. from [llvm-readobj] - Refactor the code. NFC. to [llvm-readobj] - Refactor the code..
Tue, Sep 17, 1:52 AM · Restricted Project
grimar added a comment to D67624: [llvm-readobj] - Refactor the code..

I`ll remove NFC from title. It prints a <?> as a name instead of nothing when error occurs.
(But these cases were never tested yet anyways.)

Tue, Sep 17, 1:52 AM · Restricted Project
grimar added a comment to D67617: [llvm-readobj] - Test PPC64 relocations properly..

Looks good, though, as a belated thought after D62594, I wonder if we really want to test every enum member listed in BinaryFormat/ELFRelocs/*.def. The source code just uses the X macros pattern to support every enum member, but we test every possibility.

Tue, Sep 17, 1:42 AM · Restricted Project
grimar committed rGa5dfa70806be: [llvm-objcopy] - Remove python invocations from 2 test cases. (authored by grimar).
[llvm-objcopy] - Remove python invocations from 2 test cases.
Tue, Sep 17, 1:38 AM
grimar committed rL372081: [llvm-objcopy] - Remove python invocations from 2 test cases..
[llvm-objcopy] - Remove python invocations from 2 test cases.
Tue, Sep 17, 1:37 AM
grimar closed D67610: [llvm-objcopy] - Remove python invocations from 2 test cases..
Tue, Sep 17, 1:37 AM · Restricted Project
grimar added inline comments to D67328: [llvm-readobj] Warn user when dumping not supported version of stack map (PR38278)..
Tue, Sep 17, 1:25 AM · Restricted Project

Mon, Sep 16

grimar updated the diff for D67624: [llvm-readobj] - Refactor the code..
  • Now with context.
Mon, Sep 16, 8:36 AM · Restricted Project
grimar created D67624: [llvm-readobj] - Refactor the code..
Mon, Sep 16, 8:36 AM · Restricted Project
grimar updated the diff for D67615: [obj2yaml] - Support PPC64 relocation types..
  • Simplify the YAML a bit.
Mon, Sep 16, 6:49 AM · Restricted Project
grimar created D67617: [llvm-readobj] - Test PPC64 relocations properly..
Mon, Sep 16, 6:40 AM · Restricted Project
grimar created D67615: [obj2yaml] - Support PPC64 relocation types..
Mon, Sep 16, 6:21 AM · Restricted Project
grimar added inline comments to D67547: [llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and report warnings about that..
Mon, Sep 16, 5:51 AM · Restricted Project
grimar added inline comments to D67547: [llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and report warnings about that..
Mon, Sep 16, 5:20 AM · Restricted Project
grimar updated the diff for D67547: [llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and report warnings about that..
  • Addressed review comments.
Mon, Sep 16, 3:38 AM · Restricted Project
grimar added inline comments to D67610: [llvm-objcopy] - Remove python invocations from 2 test cases..
Mon, Sep 16, 3:04 AM · Restricted Project
grimar created D67610: [llvm-objcopy] - Remove python invocations from 2 test cases..
Mon, Sep 16, 2:50 AM · Restricted Project
grimar created D67609: [llvm-readobj] - Fix a TODO in elf-reloc-zero-name-or-value.test..
Mon, Sep 16, 2:05 AM · Restricted Project

Sat, Sep 14

grimar added inline comments to D67449: [llvm-ar] Include a line number when failing to parse an MRI script.
Sat, Sep 14, 5:11 AM · Restricted Project
grimar added inline comments to D67560: [llvm-ar] Parse 'h' and '-h': display help and exit.
Sat, Sep 14, 5:05 AM · Restricted Project

Fri, Sep 13

grimar accepted D67558: [llvm-ar] Uncapitalize and delete full stop from error messages.

LGTM with a nits.

Fri, Sep 13, 9:35 AM · Restricted Project
grimar added inline comments to D67558: [llvm-ar] Uncapitalize and delete full stop from error messages.
Fri, Sep 13, 9:15 AM · Restricted Project
grimar added inline comments to D67558: [llvm-ar] Uncapitalize and delete full stop from error messages.
Fri, Sep 13, 9:10 AM · Restricted Project
grimar committed rG69ba3defafa7: [lldb] - Update unit tests after lib/ObjectYAML change. (authored by grimar).
[lldb] - Update unit tests after lib/ObjectYAML change.
Fri, Sep 13, 9:00 AM
grimar committed rG850110272783: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors… (authored by grimar).
[yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors…
Fri, Sep 13, 9:00 AM
grimar committed rL371866: [lldb] - Update unit tests after lib/ObjectYAML change..
[lldb] - Update unit tests after lib/ObjectYAML change.
Fri, Sep 13, 9:00 AM
grimar committed rL371865: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors….
[yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors…
Fri, Sep 13, 8:59 AM
grimar closed D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers..
Fri, Sep 13, 8:59 AM · Restricted Project
grimar updated the diff for D67547: [llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and report warnings about that..
  • A cosmetic cleanup.
Fri, Sep 13, 6:21 AM · Restricted Project
grimar created D67547: [llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and report warnings about that..
Fri, Sep 13, 6:20 AM · Restricted Project
grimar committed rG7da559f2f60e: [lib/ObjectYAML] - Change interface to return `bool` instead of `int`. NFCI (authored by grimar).
[lib/ObjectYAML] - Change interface to return `bool` instead of `int`. NFCI
Fri, Sep 13, 2:13 AM
grimar committed rL371828: [lib/ObjectYAML] - Change interface to return `bool` instead of `int`. NFCI.
[lib/ObjectYAML] - Change interface to return `bool` instead of `int`. NFCI
Fri, Sep 13, 2:13 AM
grimar closed D67488: [lib/ObjectYAML] - Change interface to return `bool` instead of `int`. NFCI..
Fri, Sep 13, 2:13 AM · Restricted Project
grimar committed rGd70690833908: [llvm-objdump] Fix llvm-objdump --all-headers output order (authored by grimar).
[llvm-objdump] Fix llvm-objdump --all-headers output order
Fri, Sep 13, 1:56 AM
grimar committed rL371826: [llvm-objdump] Fix llvm-objdump --all-headers output order.
[llvm-objdump] Fix llvm-objdump --all-headers output order
Fri, Sep 13, 1:55 AM
grimar closed D67357: [llvm-objdump] Fix llvm-objdump --all-headers output order.
Fri, Sep 13, 1:55 AM · Restricted Project
grimar added a comment to D67504: [ELF] Make MergeInputSection merging aware of output sections.

Generally the approach looks good to me. Lets see what other think.

Fri, Sep 13, 1:47 AM · Restricted Project
grimar accepted D67531: [ELF] Delete SectionBase::assigned.

LGTM

Fri, Sep 13, 1:29 AM · Restricted Project
grimar added inline comments to D67488: [lib/ObjectYAML] - Change interface to return `bool` instead of `int`. NFCI..
Fri, Sep 13, 1:17 AM · Restricted Project
grimar added inline comments to D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers..
Fri, Sep 13, 1:17 AM · Restricted Project
grimar added a comment to D45375: [ELF] - Introduce synthetic file for linker script symbols..

One of the latest comment from Rui about this approach was: "I'm still in doubt if this is overall a win. At least this patch itself doesn't demonstrate the benefit of having an extra type of file, and I doubt if it will be. Can this simplify things significantly? I think I'm not convinced that we want this one."
If something have changed, I can try to ressurect this patch.

Fri, Sep 13, 1:00 AM

Thu, Sep 12

grimar added a child revision for D67488: [lib/ObjectYAML] - Change interface to return `bool` instead of `int`. NFCI.: D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers..
Thu, Sep 12, 3:44 AM · Restricted Project
grimar added inline comments to D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers..
Thu, Sep 12, 3:44 AM · Restricted Project
grimar added a parent revision for D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers.: D67488: [lib/ObjectYAML] - Change interface to return `bool` instead of `int`. NFCI..
Thu, Sep 12, 3:44 AM · Restricted Project
grimar updated the diff for D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers..
  • Addressed review comments.
  • Now rebased on top of D67488.
Thu, Sep 12, 3:44 AM · Restricted Project
grimar created D67488: [lib/ObjectYAML] - Change interface to return `bool` instead of `int`. NFCI..
Thu, Sep 12, 3:44 AM · Restricted Project
grimar added a comment to D67482: [ELF][X86] Allow PT_LOAD to have overlapping p_offset ranges on EM_X86_64.

I also think it is OK. I haven't check all the math, seems there are problems with it in some places.
But seems we never cared too much about it anyways.

Thu, Sep 12, 2:18 AM · Restricted Project
grimar accepted D67357: [llvm-objdump] Fix llvm-objdump --all-headers output order.

LGTM. Thanks!

Thu, Sep 12, 12:53 AM · Restricted Project

Wed, Sep 11

grimar added inline comments to D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers..
Wed, Sep 11, 7:47 AM · Restricted Project
grimar created D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers..
Wed, Sep 11, 7:12 AM · Restricted Project
grimar added a comment to D67357: [llvm-objdump] Fix llvm-objdump --all-headers output order.

Please reupload the patch with context so that reviewers could see lines around your changes (https://llvm.org/docs/Phabricator.html#id4).

Wed, Sep 11, 1:10 AM · Restricted Project
grimar added reviewers for D67357: [llvm-objdump] Fix llvm-objdump --all-headers output order: jhenderson, MaskRay.
Wed, Sep 11, 1:10 AM · Restricted Project

Tue, Sep 10

grimar added a comment to D67388: Add a feature to dump dependency graph..

I looked at the sample provided and I think that if I was a new user, I'd find not clear what is "<internal>" probably.
What about including something like "<internal> is usually a symbol defined by linker" line to the header?

Also I am not sure what is situation now, but before we used to report "<internal>" for linker script defined symbols
in our messages sometimes. Would be nice to have a test for that case.

"<internal>" does not represent a symbol but a file. Ideally we should give more meaningful name for each virtual file, but I think that's beyond the scope of this patch.

If it reports "<internal>" for a symbol from a linker script still - it should be mentioned in the header. Otherwise it might be not clear where a sybmol is coming from.

If we want to have a meaningful file name for synthesized sections, linker synthesized symbols, and SymbolAssignment symbols, we have to create a new InputFile::Kind

std::string lld::toString(const InputFile *f) {
  if (!f)  // this can be null for various reasons above
    return "<internal>";
Tue, Sep 10, 3:05 AM · Restricted Project
grimar added a comment to D67388: Add a feature to dump dependency graph..

I looked at the sample provided and I think that if I was a new user, I'd find not clear what is "<internal>" probably.
What about including something like "<internal> is usually a symbol defined by linker" line to the header?

Also I am not sure what is situation now, but before we used to report "<internal>" for linker script defined symbols
in our messages sometimes. Would be nice to have a test for that case.

"<internal>" does not represent a symbol but a file. Ideally we should give more meaningful name for each virtual file, but I think that's beyond the scope of this patch.

Tue, Sep 10, 2:28 AM · Restricted Project
grimar added a comment to D67388: Add a feature to dump dependency graph..

I looked at the sample provided and I think that if I was a new user, I'd find not clear what is "<internal>" probably.
What about including something like "<internal> is usually a symbol defined by linker" line to the header?

Tue, Sep 10, 1:57 AM · Restricted Project
grimar added inline comments to D67328: [llvm-readobj] Warn user when dumping not supported version of stack map (PR38278)..
Tue, Sep 10, 1:47 AM · Restricted Project
grimar added inline comments to D67328: [llvm-readobj] Warn user when dumping not supported version of stack map (PR38278)..
Tue, Sep 10, 1:30 AM · Restricted Project
grimar added inline comments to D67328: [llvm-readobj] Warn user when dumping not supported version of stack map (PR38278)..
Tue, Sep 10, 1:28 AM · Restricted Project
grimar added a comment to D67284: [LLD][ELF][ARM] Implement --fix-cortex-a8 to fix erratum 657417.

Just a few nits from me.

Tue, Sep 10, 12:41 AM · Restricted Project

Mon, Sep 9

grimar committed rGc11af417e0dd: [yaml2obj] - Fix BB after r371380 (authored by grimar).
[yaml2obj] - Fix BB after r371380
Mon, Sep 9, 2:57 AM
grimar committed rL371383: [yaml2obj] - Fix BB after r371380.
[yaml2obj] - Fix BB after r371380
Mon, Sep 9, 2:57 AM