[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

labath added a comment. In D81119#2074109 , @jankratochvil wrote: > In D81119#2073973 , @labath wrote: > > > PS: You can still just drop the test if you think that's too much hassle. :) > > > It is sure nice to lea

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

jankratochvil added a comment. In D81119#2074954 , @jasonmolenda wrote: > lldb-unit :: Utility/./UtilityTests/DataExtractorTest.GetSLEB128_bit63 > lldb-shell :: SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s That should be fixed by rG846909e2a

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

jasonmolenda added a comment. Hi Jan, I noticed our sanitizer bot started getting failures in Failing Tests (3): lldb-api :: commands/expression/unwind_expression/TestUnwindExpression.py lldb-unit :: Utility/./UtilityTests/DataExtractorTest.GetSLEB128_bit63 lldb-shell :: SymbolFile/DWARF/D

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

This revision was automatically updated to reflect the committed changes. Closed by commit rG476f520a0bd2: [lldb] Fix SLEB128 decoding (authored by jankratochvil). Changed prior to commit: https://reviews.llvm.org/D81119?vs=268489&id=268535#toc Repository: rG LLVM Github Monorepo CHANGES SI

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

jankratochvil marked 5 inline comments as done. jankratochvil added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:6 +# RUN: llvm-mc -g -dwarf-version=5 -triple x86_64-unknown-linux-gnu %s -filetype=obj > %t.o +# RUN: ld.lld -m elf_x86_64 %t.o -o %

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

jankratochvil marked 12 inline comments as done. jankratochvil added a comment. In D81119#2073973 , @labath wrote: > PS: You can still just drop the test if you think that's too much hassle. :) It is sure nice to learn more the LLDB high standards, thank

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Thanks. The fix and the unittest look good. I have a bunch of comments on the .s file. As I note in the inline comments, I don't think that having a test like that for specifically handling sl

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

jankratochvil updated this revision to Diff 268489. jankratochvil added a comment. Added unit test, simplified the `.s` test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81119/new/ https://reviews.llvm.org/D81119 Files: lldb/source/Utility/Dat

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

labath added a comment. The fix is good (thanks for tracking that down), but the test is way to complicated for what the fix does. A generic "can we show the DW_AT_const_value of a global variable" test might be useful -- I don't believe we have anything exactly like that right now -- but it o

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

shafik added a comment. I would also like to see a unit test, unless the DWARF test somehow covers as aspect that a unit test would and even in that case the unit test is still important. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81119/new/ h

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

clayborg added a comment. Seems like it would be easier to add a unit test to lldb/unittests/Utility/DataExtractorTest.cpp. Then you can verify any edge case without having to make DWARF in a .s file? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

davide added a comment. Greg & Pavel might have opinions on this patch. I'm not qualified to review it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81119/new/ https://reviews.llvm.org/D81119 ___ lldb

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

jankratochvil created this revision. jankratochvil added reviewers: dblaikie, davide. jankratochvil added a project: LLDB. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. jankratochvil retitled this revision from "Fix SLEB128 decoding" to "[lldb] Fix SLEB128 decoding". dav