Page MenuHomePhabricator

grimar (George Rimar)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 21 2015, 12:36 AM (299 w, 3 d)

Recent Activity

Mar 16 2021

grimar added inline comments to D96883: Add support for JSON output style to llvm-symbolizer.
Mar 16 2021, 2:17 AM · Restricted Project

Mar 15 2021

grimar accepted D96883: Add support for JSON output style to llvm-symbolizer.

I think I can accept it (with nits fixed), so LGTM. I've added few last minor comments/notes.
(there also seems to be some issues with English grammar in comments, but it is not a place where I can help much).

Mar 15 2021, 1:01 AM · Restricted Project

Mar 12 2021

grimar added a comment to D96883: Add support for JSON output style to llvm-symbolizer.

Looks mostly good to me. I'd suggest to focus on refining the comments in test cases.
Currenty it is often not clear what is tested and what is expected result.
E.g:

Mar 12 2021, 12:37 AM · Restricted Project

Mar 10 2021

grimar added a comment to D96883: Add support for JSON output style to llvm-symbolizer.

Few more comments. I think that error reporting could be better, we should probably use
the power of Expected<> and show the real errors, rather than hiding them under standard messages for error codes.

Mar 10 2021, 4:27 AM · Restricted Project

Mar 9 2021

grimar added a comment to D96883: Add support for JSON output style to llvm-symbolizer.

I apologize for my belated feedback. I am still on vacation, but I'd like to add my 5 cents about this particular patch to try to push it forward.

Mar 9 2021, 5:12 AM · Restricted Project

Jan 30 2021

grimar committed rGd22140687500: [llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and… (authored by grimar).
[llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and…
Jan 30 2021, 7:37 AM
grimar closed D95609: [llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and missing input file..
Jan 30 2021, 7:37 AM · Restricted Project

Jan 29 2021

grimar added inline comments to D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk..
Jan 29 2021, 4:05 AM · Restricted Project
grimar updated the diff for D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk..
  • Addressed review comment.
Jan 29 2021, 4:05 AM · Restricted Project
grimar updated the diff for D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk..
  • Fix the obj2yaml crash on a binary processed by llvm-strip --remove-sections. The issue happened when there was no section header table. I've fixed it and added a test case to program-headers.yaml. Note that currently obj2yaml doesn't emit a SectionHeaderTable chunk with NoHeaders = true for such a case. But this issue is unrelated to this patch, we already have the following FIXME in the eshnum.yaml test:
Jan 29 2021, 1:50 AM · Restricted Project
grimar committed rGa5154ab9b0c1: [llvm-readobj/elf] - Report "bitcode files are not supported" warning for… (authored by grimar).
[llvm-readobj/elf] - Report "bitcode files are not supported" warning for…
Jan 29 2021, 1:05 AM
grimar closed D95605: [llvm-readobj/elf] - Report "bitcode files are not supported" warning for bitcode files..
Jan 29 2021, 1:05 AM · Restricted Project
grimar added a comment to D95158: Use DataExtractor to decode SLEB128 in android_relas..

I am surprised this patch hasn't been automatically updated to show the committed status after I pushed it (https://github.com/llvm/llvm-project/commit/3ca502a7d60787abe14db1fa541950ff79c7585b). Maybe I should've used arc land instead of git push?

Probably because you've used "Differential Revision". The latter word should be "revision"

No, it definitely should be Revision (upper-case).

Jan 29 2021, 12:47 AM · Restricted Project
grimar added a comment to D95609: [llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and missing input file..

LGTM. I've also approved D95246. That's a larger change, so perhaps let that land first, if you can then rebase and commit. If it hasn't landed before you need to be stopping work, feel free to land this first though, given you're away soon.

Jan 29 2021, 12:44 AM · Restricted Project
grimar added a comment to D95158: Use DataExtractor to decode SLEB128 in android_relas..

I am surprised this patch hasn't been automatically updated to show the committed status after I pushed it (https://github.com/llvm/llvm-project/commit/3ca502a7d60787abe14db1fa541950ff79c7585b). Maybe I should've used arc land instead of git push?

Jan 29 2021, 12:25 AM · Restricted Project
grimar updated the diff for D95609: [llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and missing input file..
  • Addressed review comments.
Jan 29 2021, 12:07 AM · Restricted Project

Jan 28 2021

grimar updated the summary of D95609: [llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and missing input file..
Jan 28 2021, 11:59 PM · Restricted Project
grimar updated the summary of D95609: [llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and missing input file..
Jan 28 2021, 11:58 PM · Restricted Project
grimar added a comment to D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk..

Program headers are not subsidiary of the section header table. ProgramHeaderType is fine, but it probably should not have Segments:. The program headers can still be described the current ProgramHeaders:.

Jan 28 2021, 11:56 PM · Restricted Project
grimar added inline comments to D95605: [llvm-readobj/elf] - Report "bitcode files are not supported" warning for bitcode files..
Jan 28 2021, 6:28 AM · Restricted Project
grimar updated the diff for D95605: [llvm-readobj/elf] - Report "bitcode files are not supported" warning for bitcode files..
  • Addressed review comments.
Jan 28 2021, 6:28 AM · Restricted Project
grimar added inline comments to D95609: [llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and missing input file..
Jan 28 2021, 5:56 AM · Restricted Project
grimar updated the summary of D95609: [llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and missing input file..
Jan 28 2021, 5:47 AM · Restricted Project
grimar requested review of D95609: [llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and missing input file..
Jan 28 2021, 5:47 AM · Restricted Project
grimar added inline comments to D95605: [llvm-readobj/elf] - Report "bitcode files are not supported" warning for bitcode files..
Jan 28 2021, 5:34 AM · Restricted Project
grimar added inline comments to D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk..
Jan 28 2021, 5:28 AM · Restricted Project
grimar added inline comments to D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk..
Jan 28 2021, 4:39 AM · Restricted Project
grimar updated the diff for D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk..
  • Addressed review comments.
  • Improved testing.
Jan 28 2021, 4:39 AM · Restricted Project
grimar added a comment to D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk..

I might well have missed it, but I don't recall seeing any dedicated obj2yaml tests?

Jan 28 2021, 3:17 AM · Restricted Project
grimar requested review of D95605: [llvm-readobj/elf] - Report "bitcode files are not supported" warning for bitcode files..
Jan 28 2021, 2:58 AM · Restricted Project
grimar added inline comments to D95511: [llvm-readelf] Support dumping the BB address map section with --bb-addr-map..
Jan 28 2021, 2:56 AM · Restricted Project

Jan 27 2021

grimar committed rG68195b15a36c: [yaml2obj] - Allow empty SectionHeaderTable definitions. (authored by grimar).
[yaml2obj] - Allow empty SectionHeaderTable definitions.
Jan 27 2021, 11:52 PM
grimar closed D95341: [yaml2obj] - Allow empty SectionHeaderTable definitions..
Jan 27 2021, 11:52 PM · Restricted Project
grimar requested review of D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk..
Jan 27 2021, 10:57 PM · Restricted Project
grimar added a comment to D95341: [yaml2obj] - Allow empty SectionHeaderTable definitions..

LGTM.

Hi guys.
I am sorry for pinging this too early, but I am planning a ~6 weeks vacation starting from 1st of february
and I wonder if there is something I can do for this to land it or if there are any concerns about this change?

No problem. Somehow this slipped off my review pile, so I hadn't seen it. Let me know if there are any other reviews I haven't done by the end of the day.

Jan 27 2021, 1:50 AM · Restricted Project
grimar added inline comments to D95461: [llvm-nm] Display defined weak STT_GNU_IFUNC symbols as 'i'.
Jan 27 2021, 12:22 AM · Restricted Project

Jan 26 2021

grimar added a comment to D95341: [yaml2obj] - Allow empty SectionHeaderTable definitions..

Hi guys.
I am sorry for pinging this too early, but I am planning a ~6 weeks vacation starting from 1st of february
and I wonder if there is something I can do for this to land it or if there are any concerns about this change?

Jan 26 2021, 11:48 PM · Restricted Project
grimar committed rGd5e48f1347d5: [yaml2obj][obj2yaml] - Improve how we set/dump the sh_entsize field. (authored by grimar).
[yaml2obj][obj2yaml] - Improve how we set/dump the sh_entsize field.
Jan 26 2021, 2:33 AM
grimar closed D95364: [yaml2obj][obj2yaml] - Improve how we set/dump the sh_entsize field..
Jan 26 2021, 2:33 AM · Restricted Project
grimar added inline comments to D67137: [llvm-objcopy] Implement --only-keep-debug for ELF.
Jan 26 2021, 2:10 AM · Restricted Project
grimar added a comment to D95354: [yaml2obj] - Refine how we set the sh_link field. NFCI..

It turns out this wasn't NFC, one of LLDB test cases started to crash.
I've fixed it in rG2a33b092f5b175a21680b3980ff2019bc1c65b12.

The difference was that before this patch we set the sh_link for symbol tables basing on their names (E.g ".symtab"),
now we set this field basing on section types (e.g SHT_SYMTAB).
The mentioned test case has a .text section of type SHT_SYMTAB and it did not expect to see a sh_link field set it seeems.

Based on my casual reading of that test, I wonder if you found a bug in the tool there - it looks like the intent is for section types to trump section names, which surely should mean the use of sh_link would be correctin this case? Perhaps we should highlight this to the LLDB developers (pinging @JDevlieghere for example).

Jan 26 2021, 2:04 AM · Restricted Project
grimar added a comment to D95364: [yaml2obj][obj2yaml] - Improve how we set/dump the sh_entsize field..

E.g. it seems that the expected behavior of llvm-objcopy would be to drop the sh_link for symbol table in this case. Because it is of SHT_NOBITS type.
Perhaps I'd wait from some input from @MaskRay as he was the author of the test case I think.

Jan 26 2021, 1:59 AM · Restricted Project
grimar added a comment to D95364: [yaml2obj][obj2yaml] - Improve how we set/dump the sh_entsize field..

Looks reasonable, but perhaps fixing the llvm-objcopy issue first would be best. What do you think?

Jan 26 2021, 1:50 AM · Restricted Project
grimar added a comment to D95354: [yaml2obj] - Refine how we set the sh_link field. NFCI..

It turns out this wasn't NFC, one of LLDB test cases started to crash.
I've fixed it in rG2a33b092f5b175a21680b3980ff2019bc1c65b12.

Jan 26 2021, 1:48 AM · Restricted Project
grimar committed rG2a33b092f5b1: [LLDB][test] - Fix test after yaml2obj change. (authored by grimar).
[LLDB][test] - Fix test after yaml2obj change.
Jan 26 2021, 1:43 AM
grimar committed rG029644ee5107: [yaml2obj] - Refine how we set the sh_link field. NFCI. (authored by grimar).
[yaml2obj] - Refine how we set the sh_link field. NFCI.
Jan 26 2021, 1:28 AM
grimar closed D95354: [yaml2obj] - Refine how we set the sh_link field. NFCI..
Jan 26 2021, 1:28 AM · Restricted Project
grimar committed rGdb92d47cf70e: [llvm-nm][ELF] - Use @@ prefix when printing default versions. (authored by grimar).
[llvm-nm][ELF] - Use @@ prefix when printing default versions.
Jan 26 2021, 1:17 AM
grimar closed D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Jan 26 2021, 1:17 AM · Restricted Project
grimar added inline comments to D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Jan 26 2021, 1:16 AM · Restricted Project
grimar committed rGe98d5c31925d: [libObject,llvm-readelf/obj] - Don't use @@ when printing versions of undefined… (authored by grimar).
[libObject,llvm-readelf/obj] - Don't use @@ when printing versions of undefined…
Jan 26 2021, 1:06 AM
grimar closed D95219: [libObject,llvm-readelf/obj] - Don't use @@ when printing versions of undefined symbols..
Jan 26 2021, 1:06 AM · Restricted Project
grimar added a comment to D95246: [test] Use host platform specific error message substitution in lit tests .

I like this approach.

Jan 26 2021, 12:24 AM · Restricted Project, Restricted Project, Restricted Project

Jan 25 2021

grimar requested review of D95364: [yaml2obj][obj2yaml] - Improve how we set/dump the sh_entsize field..
Jan 25 2021, 7:28 AM · Restricted Project
grimar requested review of D95354: [yaml2obj] - Refine how we set the sh_link field. NFCI..
Jan 25 2021, 5:56 AM · Restricted Project
grimar requested review of D95341: [yaml2obj] - Allow empty SectionHeaderTable definitions..
Jan 25 2021, 3:47 AM · Restricted Project
grimar committed rG19245b781576: [ObjectYAML] - An attempt to fix BB after commit of D95140. (authored by grimar).
[ObjectYAML] - An attempt to fix BB after commit of D95140.
Jan 25 2021, 2:26 AM
grimar committed rG9c89dcf80736: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk. (authored by grimar).
[yaml2obj, obj2yaml] - Implement section header table as a special Chunk.
Jan 25 2021, 2:09 AM
grimar closed D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk..
Jan 25 2021, 2:08 AM · Restricted Project
grimar updated the diff for D95219: [libObject,llvm-readelf/obj] - Don't use @@ when printing versions of undefined symbols..
  • Addressed review comment.
Jan 25 2021, 1:52 AM · Restricted Project
grimar added a comment to D95246: [test] Use host platform specific error message substitution in lit tests .

As far I understand, looking on the description of D94239, the message on z/OS looks like "EDC5129I No such file or directory.".
I guess the EDC5129I is a stable error code? So why not to check for a possible EDC5129I prefix word instead of .*?
(The same applies for other possible errors)

Jan 25 2021, 1:32 AM · Restricted Project, Restricted Project, Restricted Project
grimar added a comment to D95158: Use DataExtractor to decode SLEB128 in android_relas..

A minor nit from me, the rest looks fine.

Jan 25 2021, 1:23 AM · Restricted Project
grimar added a reviewer for D95199: [ELF] Improve compatibility with ld.bfd linker scripts: psmith.
Jan 25 2021, 1:05 AM · Restricted Project
grimar added a comment to D95199: [ELF] Improve compatibility with ld.bfd linker scripts.

This looks to me it solves a specific corner case, but I really don't feel I have enough knowledges about embedded world/scripts to say whether it is a good or bad thing to support it.
It seems matches to what GNU ld/gold do. I'll add Peter who I guess might have an opinion about this.

Jan 25 2021, 1:05 AM · Restricted Project

Jan 22 2021

grimar added inline comments to D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk..
Jan 22 2021, 5:30 AM · Restricted Project
grimar updated the diff for D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk..
  • Addressed review comments.
Jan 22 2021, 5:30 AM · Restricted Project
grimar abandoned D93678: [yaml2obj] - Support selecting the location of the section header table..

D95140 was posted instead.

Jan 22 2021, 4:53 AM · Restricted Project
grimar updated the diff for D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
  • Addressed review comments.
Jan 22 2021, 4:50 AM · Restricted Project
grimar added inline comments to D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Jan 22 2021, 3:57 AM · Restricted Project
grimar added inline comments to D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Jan 22 2021, 3:56 AM · Restricted Project
grimar requested review of D95219: [libObject,llvm-readelf/obj] - Don't use @@ when printing versions of undefined symbols..
Jan 22 2021, 3:28 AM · Restricted Project
grimar added a comment to D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk..

It feels like there needs to be testing there for writing the section header table in the right place in the YAML (with Offset if needed), but I don't see that yet.

Jan 22 2021, 1:46 AM · Restricted Project

Jan 21 2021

grimar added inline comments to D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Jan 21 2021, 11:13 PM · Restricted Project
grimar requested review of D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk..
Jan 21 2021, 7:45 AM · Restricted Project
grimar accepted D95124: [lld][ELF][test] Add testing for IE/LD TLS weak undef references.

LGTM with a nit, please wait for others.

Jan 21 2021, 4:54 AM · Restricted Project
grimar added a comment to D95124: [lld][ELF][test] Add testing for IE/LD TLS weak undef references.

missing check-prefix. Not sure how I didn't spot the latter...

Jan 21 2021, 4:42 AM · Restricted Project
grimar added inline comments to D95124: [lld][ELF][test] Add testing for IE/LD TLS weak undef references.
Jan 21 2021, 4:16 AM · Restricted Project
grimar committed rGdd5c98280473: [llvm-nm][ELF] - Make -D display symbol versions. (authored by grimar).
[llvm-nm][ELF] - Make -D display symbol versions.
Jan 21 2021, 12:25 AM
grimar closed D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Jan 21 2021, 12:24 AM · Restricted Project

Jan 20 2021

grimar committed rG51f4958057d6: [yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections. (authored by grimar).
[yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections.
Jan 20 2021, 11:52 PM
grimar closed D94956: [yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections..
Jan 20 2021, 11:51 PM · Restricted Project
grimar accepted D94560: [ELF] report section sizes when output file too large.

This LGTM, but please wait for others.

Jan 20 2021, 11:29 PM · Restricted Project
grimar added a comment to D94907: [llvm-nm][ELF] - Make -D display symbol versions..

@MaskRay, are you OK with it?

Jan 20 2021, 12:32 AM · Restricted Project

Jan 19 2021

grimar added a comment to D93761: [libObject/Decompressor] - Use `resize_for_overwrite` in Decompressor::resizeAndDecompress()..

Ping.

Jan 19 2021, 7:05 AM · Restricted Project
grimar updated the diff for D94956: [yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections..
  • Added test cases.
Jan 19 2021, 7:05 AM · Restricted Project
grimar added inline comments to D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Jan 19 2021, 6:11 AM · Restricted Project
grimar updated the diff for D94907: [llvm-nm][ELF] - Make -D display symbol versions..
  • Addressed review comment.
Jan 19 2021, 6:06 AM · Restricted Project
grimar added inline comments to D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Jan 19 2021, 5:51 AM · Restricted Project
grimar added inline comments to D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Jan 19 2021, 2:26 AM · Restricted Project
grimar updated the diff for D94907: [llvm-nm][ELF] - Make -D display symbol versions..
  • Addressed review comments.
Jan 19 2021, 2:26 AM · Restricted Project
grimar requested review of D94956: [yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections..
Jan 19 2021, 2:05 AM · Restricted Project

Jan 18 2021

grimar requested review of D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Jan 18 2021, 5:44 AM · Restricted Project
grimar updated the diff for D94907: [llvm-nm][ELF] - Make -D display symbol versions..
  • Format.
Jan 18 2021, 4:33 AM · Restricted Project
grimar added inline comments to D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Jan 18 2021, 4:31 AM · Restricted Project
grimar requested review of D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Jan 18 2021, 4:29 AM · Restricted Project
grimar committed rGb9ce772b8fb5: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h (authored by grimar).
[Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h
Jan 18 2021, 1:51 AM
grimar closed D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.
Jan 18 2021, 1:50 AM · Restricted Project
grimar updated the diff for D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.
  • Addressed review comments.
Jan 18 2021, 12:58 AM · Restricted Project
grimar updated the summary of D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.
Jan 18 2021, 12:54 AM · Restricted Project
grimar added inline comments to D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.
Jan 18 2021, 12:49 AM · Restricted Project