Harshit Sharma has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42496 )
Change subject: lib: Add ASan support to ramstage on x86 arch ......................................................................
Patch Set 20:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42496/19/src/include/asan.h File src/include/asan.h:
https://review.coreboot.org/c/coreboot/+/42496/19/src/include/asan.h@4 PS19, Line 4:
I would prefer types.h here.
Done
https://review.coreboot.org/c/coreboot/+/42496/19/src/include/asan.h@5 PS19, Line 5: &_shadow
I would honestly rather see &_shadow in the code so I don't have to lookup what the macro is.
Done
https://review.coreboot.org/c/coreboot/+/42496/19/src/include/asan.h@8 PS19, Line 8: ASAN_SHADOW_OFFSET (((uint32_t) &_shadow) -\ : (((uint32_t) &_data) >> ASAN_SHADOW_SCALE_SHIFT))
I would rather see this in code with some documentation on the expectations.
Done
https://review.coreboot.org/c/coreboot/+/42496/19/src/include/asan.h@50 PS19, Line 50: void
void? I'd expect a name to be of char type (void pointers and char pointers are equivalent, I think)
I took it from Linux's KASAN. They have used void. But as you said, they should be equivalent.
https://review.coreboot.org/c/coreboot/+/42496/19/src/lib/asan.c File src/lib/asan.c:
https://review.coreboot.org/c/coreboot/+/42496/19/src/lib/asan.c@26 PS19, Line 26: asan_mem_to_shadow
I guess I'm not really clear what this does... […]
This function returns the corresponding shadow address for addr in memory. By mem we mean any address which belongs to our program.