[PATCH] D139184: [LLD][Windows]Feature "checksum" for Windows PE

Wed Dec 28 13:59:30 PST 2022

mstorsjo added a comment.

In D139184#4016943 <https://reviews.llvm.org/D139184#4016943>, @aganea wrote:

> If someone was to take a look at that code, they would see it is different from what it is currently written in this patch.

Thanks for adding that context info!

> The algorithm used for the PE checksum is essentially RFC1071 <http://www.faqs.org/rfcs/rfc1071.html>, as documented here <https://www.codeproject.com/Articles/19326/An-Analysis-of-the-Windows-PE-Checksum-Algorithm> and here <https://practicalsecurityanalytics.com/pe-checksum/>. We already have an implementation in libc, here <https://github.com/llvm/llvm-project/blob/2e999b7dd1934a44d38c3a753460f1e5a217e9a5/libc/AOR_v20.02/networking/test/chksum.c#L40>, but I wouldn't touch or move it.

Thanks!

> I think the whole algorithm can be implemented more simply just as it is described in RFC1071:
>
>   void Writer::writePEChecksum() {
>   [...]
>     uint16_t *addr = (uint16_t *)buf;

I think this needs to use `ulittle32_t *addr` instead (as the images are written in that form anyway), so that we get the same checksum when running the linker on big-endian hosts. (I would expect that we do have some buildbot configs that run on big endian.)

> And probably a comment that mentions a link to the RFC.

Indeed, please add at least a mention of RFC1071! And we do need a testcase too, but I see that you're working on the prerequisites for that in D140555 <https://reviews.llvm.org/D140555>.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139184