Raul Rangel 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 19:
(4 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@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.
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.
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... What is ASAN mem? I would assume that's in the shadow region?
https://review.coreboot.org/c/coreboot/+/42496/19/src/lib/program.ld File src/lib/program.ld:
https://review.coreboot.org/c/coreboot/+/42496/19/src/lib/program.ld@130 PS19, Line 130: .shadow . : { : _shadow_size = (_eheap - _data) >> 3; : . = ALIGN(ARCH_POINTER_ALIGN_SIZE); : _shadow = .; : . += _shadow_size; : . = ALIGN(ARCH_POINTER_ALIGN_SIZE); : _eshadow = .; : } : #endif How about:
_shadow_size = (_eheap - _data) >> 3; REGION(asan_shadow, ., _shadow_size, ARCH_POINTER_ALIGN_SIZE)