While trying to update the WebKitGTK+ packaging in Buildroot to the recent 2.20.0 release I've found that the following prevents from building for 32-bit ARM (more precisely, this was targeting ARMv7): In file included from ../../bmalloc/bmalloc/HeapKind.h:30:0, from ../../bmalloc/bmalloc/ObjectType.h:30, from ../../bmalloc/bmalloc/BumpAllocator.h:31, from ../../bmalloc/bmalloc/Allocator.h:30, from ../../bmalloc/bmalloc/Cache.h:29, from ../../bmalloc/bmalloc/bmalloc.h:29, from FastMalloc.cpp:246: ../../bmalloc/bmalloc/Gigacage.h: In function ‘Gigacage::BasePtrs& Gigacage::basePtrs()’: ../../bmalloc/bmalloc/Gigacage.h:136:59: warning: cast from ‘char*’ to ‘Gigacage::BasePtrs*’ increases required alignment of target type [-Wcast-align] return *reinterpret_cast<BasePtrs*>(g_gigacageBasePtrs); ^
Created attachment 335746 [details] Patch
Attachment 335746 [details] did not pass style-queue: ERROR: Source/bmalloc/ChangeLog:3: Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: invalid cast [changelog/unwantedsecurityterms] [3] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 335746 [details] Patch Surely this does not actually fix the alignment problem...?
(In reply to Michael Catanzaro from comment #3) > Comment on attachment 335746 [details] > Patch > > Surely this does not actually fix the alignment problem...? I had first: return *reinterpret_cast<BasePtrs*>(static_cast<void*>(g_gigacageBasePtrs)); but the simpler on seemed to work. I'll do one clean build, clearing ccache, as today Carlos Lopez has found a footgun in Buildroot's ccache handling and it could be that the build succeeded when it shouldn't. I'll remove the cq+ in the meanwhile.
Comment on attachment 335746 [details] Patch Attachment 335746 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/6943489 New failing tests: stress/ftl-put-by-id-setter-exception-interesting-live-state.js.dfg-eager-no-cjit-validate
(In reply to Adrian Perez from comment #4) > (In reply to Michael Catanzaro from comment #3) > > Comment on attachment 335746 [details] > > Patch > > > > Surely this does not actually fix the alignment problem...? > > I had first: > > return > *reinterpret_cast<BasePtrs*>(static_cast<void*>(g_gigacageBasePtrs)); > > but the simpler on seemed to work. I'll do one clean build, clearing > ccache, as today Carlos Lopez has found a footgun in Buildroot's ccache > handling and it could be that the build succeeded when it shouldn't. > I'll remove the cq+ in the meanwhile. Yep, it was a bad ccache case (╯°□°）╯︵ ┻━┻ I'l re-upload the good version tomorrow.
*Poke.*
(In reply to Adrian Perez from comment #0) > While trying to update the WebKitGTK+ packaging in Buildroot to the > recent 2.20.0 release I've found that the following prevents from > building for 32-bit ARM (more precisely, this was targeting ARMv7): > > In file included from ../../bmalloc/bmalloc/HeapKind.h:30:0, > from ../../bmalloc/bmalloc/ObjectType.h:30, > from ../../bmalloc/bmalloc/BumpAllocator.h:31, > from ../../bmalloc/bmalloc/Allocator.h:30, > from ../../bmalloc/bmalloc/Cache.h:29, > from ../../bmalloc/bmalloc/bmalloc.h:29, > from FastMalloc.cpp:246: > ../../bmalloc/bmalloc/Gigacage.h: In function ‘Gigacage::BasePtrs& > Gigacage::basePtrs()’: > ../../bmalloc/bmalloc/Gigacage.h:136:59: warning: cast from ‘char*’ to > ‘Gigacage::BasePtrs*’ increases required alignment of target type > [-Wcast-align] > return *reinterpret_cast<BasePtrs*>(g_gigacageBasePtrs); > ^ Based on this, I'm quite sure your fix is incorrect. The definition of g_gigacageBasePtrs is as follows: extern "C" BEXPORT char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE] __attribute__((aligned(GIGACAGE_BASE_PTRS_SIZE))); where ... #define GIGACAGE_BASE_PTRS_SIZE 4096 and ... struct BasePtrs { void* primitive; void* jsValue; void* string; }; That is: g_gigacageBasePtrs is expected to be aligned on a 4096 boundary. Why would it increase alignment of the target type if we cast it to a BasePtrs* which points to a 12 byte type. Hence, your real issue here is that your allocation of g_gigacageBasePtrs is not aligned on 4096 bytes. Even if you silence the compiler warning, you're not fixing the real bug, and will get unexpected failures later during code execution.
(In reply to Mark Lam from comment #8) > Even if you silence the compiler warning, you're not fixing the real bug, > and will get unexpected failures later during code execution. See protectGigacageBasePtrs() and unprotectGigacageBasePtrs(). The code expects to be able to mprotect g_gigacageBasePtrs. If g_gigacageBasePtrs is not aligned on a page boundary, then you'll not get the behavior the code expects.
(In reply to Mark Lam from comment #8) > (In reply to Adrian Perez from comment #0) > > While trying to update the WebKitGTK+ packaging in Buildroot to the > > recent 2.20.0 release I've found that the following prevents from > > building for 32-bit ARM (more precisely, this was targeting ARMv7): > > > > In file included from ../../bmalloc/bmalloc/HeapKind.h:30:0, > > from ../../bmalloc/bmalloc/ObjectType.h:30, > > from ../../bmalloc/bmalloc/BumpAllocator.h:31, > > from ../../bmalloc/bmalloc/Allocator.h:30, > > from ../../bmalloc/bmalloc/Cache.h:29, > > from ../../bmalloc/bmalloc/bmalloc.h:29, > > from FastMalloc.cpp:246: > > ../../bmalloc/bmalloc/Gigacage.h: In function ‘Gigacage::BasePtrs& > > Gigacage::basePtrs()’: > > ../../bmalloc/bmalloc/Gigacage.h:136:59: warning: cast from ‘char*’ to > > ‘Gigacage::BasePtrs*’ increases required alignment of target type > > [-Wcast-align] > > return *reinterpret_cast<BasePtrs*>(g_gigacageBasePtrs); > > ^ > > Based on this, I'm quite sure your fix is incorrect. The definition of > g_gigacageBasePtrs is as follows: > > extern "C" BEXPORT char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE] > __attribute__((aligned(GIGACAGE_BASE_PTRS_SIZE))); > > where ... > > #define GIGACAGE_BASE_PTRS_SIZE 4096 > > and ... > > struct BasePtrs { > void* primitive; > void* jsValue; > void* string; > }; > > That is: g_gigacageBasePtrs is expected to be aligned on a 4096 boundary. > Why would it increase alignment of the target type if we cast it to a > BasePtrs* which points to a 12 byte type. Hence, your real issue here is > that your allocation of g_gigacageBasePtrs is not aligned on 4096 bytes. > Even if you silence the compiler warning, you're not fixing the real bug, > and will get unexpected failures later during code execution. Is it because g_gigacageBasePtrs are `char` array? I think the compiler says alignment of `char` and `BasePtrs` are different.
In case it helps: I have made a build with a more recent WebKit source and seems to work fine after r230380 was landed by Yusuke for bug #18350. So maybe we can close this one as duplicate. I had for a bit a patch with a double cast —to “void*”, then to “BasePtrs*”— sitting on my lap for a bit, but I didn't manage to wrap my head around why that works, and therefore decided to not submit it.
Sounds like this is fixed, if the build is no longer failing and the warning is gone?
This was actually resolved by Yusuke's alignas patch which gave g_gigacageBasePtrs the alignment it needs. *** This bug has been marked as a duplicate of bug 183508 ***