[PATCH] D86422: [ELF] .note.gnu.property: error for invalid pr_datasize

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 24 03:01:02 PDT 2020


grimar added inline comments.


================
Comment at: lld/ELF/InputFiles.cpp:818
+      if (desc.size() < 8 + size)
+        reportFatal(desc, "program property is too short");
 
----------------
grimar wrote:
> 2 points:
> 
> 1) You should be able to use `read32` from `Target.h`:
> 
> ```
> inline uint32_t read32(const void *p) {
>   return llvm::support::endian::read32(p, config->endianness);
> }
> ```
> 
> 2) It probably would read slightly better if it was:
> 
> ```
> if (desc.size() < 8)
>   reportFatal(...);
> 
> uint32_t type = read32(desc.data());
> uint32_t size = read32(desc.data() + 4);
> desc = desc.drop_front(8);
> 
> if (desc.size() < size)
>   reportFatal(desc, "program property is too short");
> ```
> 
> i.e
> a) I'd not start reading the `size` + `type` when the data is truncated (just like the code on the left).
> b) Early dropping of 8 bytes at start should allow to avoid the need of `+ 8` in the code below.
> I'd not start reading the size + type when the data is truncated

The code indeed does not starts reading them, I meant I'd just return earlier when it is already known that we can't read the header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86422



More information about the llvm-commits mailing list