[PATCH] D84913: [libFuzzer] Enable for SystemZ
Ilya Leoshkevich via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 30 09:28:12 PDT 2020
iii added inline comments.
================
Comment at: compiler-rt/lib/fuzzer/FuzzerTracePC.h:198
+ if (LargeType Bundle = *reinterpret_cast<const LargeType *>(P)) {
+#if __BYTE_ORDER == __BIG_ENDIAN
+ Bundle = Bswap(Bundle);
----------------
kcc wrote:
> iii wrote:
> > kcc wrote:
> > > please avoid #ifdefs inside functions.
> > > If there is some logic that needs to depend on the platform, it needs to reside in a separate dedicated function.
> > >
> > > FuzzerBuiltins.h is probably the place for such a function.
> > >
> > > Also, what about
> > > #include <endian.h>
> > > uint64_t htole64(uint64_t host_64bits);
> > > is that portable?
> > >
> > I think this might not work as expected on 32-bit systems. I'll add a separate function.
> No strong preference.
> I've just checked, on Linux x86_64: it inlines and works great.
> Dunno if it will have portability issues.
>
> ```
> % cat x.cc
> #include <endian.h>
> #include <stdint.h>
>
> uint64_t foo(uint64_t x) {
> return htole64(x);
> }
> % clang -O2 -S -o - x.cc -m32
> movl 4(%esp), %eax
> movl 8(%esp), %edx
> retl
>
> % clang -O2 -S -o - x.cc
>
> movq %rdi, %rax
> retq
> ```
>
> `htobe64` also looks nice, it translates into one `bswapq` on 64-bit and two `bswapl` on 32-bit,
> i.e. in your case this is what `htole64` will do.
My thinking is that LargeType is uintptr_t, so it will be uint32_t on a 32-bit system.
So if we have something like
uint32_t Bundle = 0xAABBCCDD;
htole64(Bundle) would give us (uint64_t)0xDDCCBBAA00000000, and then when we cast it back to uint32_t, we'll get just 0. Isn't that right?
So the idea I'm currently regtesting is:
```
--- a/compiler-rt/lib/fuzzer/FuzzerUtil.h
+++ b/compiler-rt/lib/fuzzer/FuzzerUtil.h
@@ -106,6 +106,12 @@ inline uint8_t *RoundDownByPage(uint8_t *P) {
return reinterpret_cast<uint8_t *>(X);
}
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+template <typename T> T HToLE(T X) { return X; }
+#else
+template <typename T> T HToLE(T X) { return Bswap(X); }
+#endif
+
} // namespace fuzzer
#endif // LLVM_FUZZER_UTIL_H
```
... it's in FuzzerUtil.h and not in FuzzerBuiltins.h, because it uses Bswap, which is defined in a separate header for MSVC.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84913/new/
https://reviews.llvm.org/D84913
More information about the llvm-commits
mailing list