crashappsec / libcon4m

Base Compiler and Runtime Support for con4m
Apache License 2.0
0 stars 0 forks source link

Make a first pass at cleaning up hatrack's public headers #19

Closed orangematt closed 3 months ago

orangematt commented 3 months ago

There's still more work to be done here, but I didn't want this to get too big. My primary intent was to clean up the headers so that they each properly compile on their own without circular references involving hatrack.h. But when I looked at hatrack.h I realized that I had to fix the malloc problem first.

As Hatrack was initially written, it used libc malloc APIs to allocate memory. Using malloc / calloc for Hatrack's needs has its own issues which I will get to in a minute, but more important here is that Integrating it into libcon4m, we want it to use con4m's memory API instead. Let's just say that doing this by way of preprocessor macros in hatrack.h is not the best way. I've introduced an abstraction to Hatrack to allow clients to specify memory allocation functions to use instead. They default to use the equivalent libc functions. A large portion of the changes in this PR is all related to this, mainly switching call sites to use the new Hatrack versions.

In many places, Hatrack has strict alignment requirements in order to use DCAS CPU instructions. If we always require allocations to be 16-byte aligned and use alignas(16) as needed, this alignment requirement is satisfied for aarch64 and x86-64. I've removed most of the alignas directives that were present before because they were either unnecessary or didn't actually do what was intended. I suspect that the main issue that lead to all of this is that malloc/calloc/realloc only guarantee 8-byte alignment, though I have this weird recollection that macOS guarantees 16-byte alignment, but I may be imagining that; I haven't gone looking to verify. The Hatrack malloc API introduces the requirement that allocations must be 16-byte aligned. The API currently does not provide for a caller to specify the desired alignment. I suspect this may be desirable for at least cache-line aligned allocations in the future, but at least for today, it doesn't appear that any effort has been made in Hatrack to optimize for processor cache-lines. There were a couple of instances of alignas(64), which I assume were meant to be cache-line alignment, but they had no effect and have been removed. And anyway, 64 is no longer correct for all common hardware. Apple Silicon uses 128-byte cache lines.

The rest of the changes are to make all of the public Hatrack header files compile cleanly. The hatrack.h umbrella header is free of anything other than includes of other Hatrack headers and should stay that way. Anything that should be available to all Hatrack headers should go into hatrack/base.h and individual Hatrack headers should include base.h and any other headers that it requires in order to compile cleanly. Inclusion of libc header files should be kept to a minimum. Several unnecessary headers previously included by hatrack.h were removed. Ideally it would be nice if the headers could satisfy IWYU rules, and they almost do -- base.h I think is the only thing that violates them for now.

There's still a mix of public/private in the Hatrack headers that should be separated, and other changes will be necessary to facilitate using Hatrack in other projects, but this is a good start.