[PATCH] D144565: dwp check overflow

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang added a comment.

Some minor comments.



================
Comment at: llvm/lib/DWP/DWP.cpp:192
+  StringRef SectionName) {
+  std::string Msg = SectionName.str() + std::string(" Section Contribution Offset overflow 4G. Previous Offset ")
+    + std::to_string(PrevOffset) + std::string(", After overflow offset ") 
----------------
Do you format the changes ?


================
Comment at: llvm/lib/DWP/DWP.cpp:193
+  std::string Msg = SectionName.str() + std::string(" Section Contribution Offset overflow 4G. Previous Offset ")
+    + std::to_string(PrevOffset) + std::string(", After overflow offset ") 
+    + std::to_string(OverflowedOffset) + std::string(".");
----------------
I would prefer to use the StringRef to concat them, and then, convert base to std::string.


================
Comment at: llvm/lib/DWP/DWP.cpp:238
+      Error Err (sectionOverflowErrorOrWarning(OldOffset, TypesOffset, "Types"));
+      if (Err) {
+        return Err;
----------------
Please remove the {} if there is only one statement in the if clause. The same rule applies in other changes.


================
Comment at: llvm/lib/DWP/DWP.cpp:728-729
+          }
+            // return make_error<DWPError>(
+            //     "debug information section offset is greater than 4GB");
 
----------------
Remove this comments.


================
Comment at: llvm/test/tools/llvm-dwp/Inputs/overflow/debug_info.s:79
+.Lskel_string0:
+	.asciz	"/data00/home/zhuna.1024/gdb-10.1/hello" # string offset=0
+.Lskel_string1:
----------------
remove such kind of information


================
Comment at: llvm/test/tools/llvm-dwp/X86/overflow_debug_info.test:2
+RUN: llvm-mc --triple=x86_64-unknown-linux --filetype=obj --split-dwarf-file=debug_info.dwo -dwarf-version=5 %p/../Inputs/overflow/debug_info.s -o debug_info.o
+RUN: llvm-mc --triple=x86_64-unknown-linux --filetype=obj --split-dwarf-file=main.dwo -dwarf-version=5 %p/../Inputs/overflow/main.s -o main.o
+RUN: not llvm-dwp -e debug_info.o -e main.o -o overflow.dwp 2>&1 | FileCheck %s
----------------
How about the dwarf 4 ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144565/new/

https://reviews.llvm.org/D144565