[PATCH] D151755: [DebugInfo] Add error checking around data extraction in DWARFAbbreviationDeclaration::extract

Felipe de Azevedo Piovezan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 31 04:19:33 PDT 2023


fdeazeve added inline comments.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp:45
 
+void writeBadAbbreviationDeclarationData(raw_ostream &OS, size_t Size,
+                                         uint32_t Data) {
----------------
Small nit but it would be helpful for readers if we improve the naming here a bit. There is nothing inherently "Bad" or inherently "Abbreviation" in the function, this is a form of `write_n` algorithm.

Is it bad because we always write an odd number of elements? Or is it bad because we never have the proper zero terminators? Or because the contents of `Data`, when splat Size times, would fail to be decoded as an ULEB? I think the answer to this is related to @jhenderson's suggestion for different tests

In all those cases, my suggestion would be to rename this to `writeValueNTimes` and then on the callee side we name the data/size to `BadData` and `BadSize` and maybe add a comment on why it's bad


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151755



More information about the llvm-commits mailing list