| Message ID | 56178638.4030601@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On 10/09/2015 03:17 AM, Ramana Radhakrishnan wrote: > This started as a Friday afternoon project ... > > It turned out enabling VTV for AArch64 and ARM was a matter of fixing > PR67868 which essentially comes from building libvtv with section > anchors turned on. The problem was that the flow of control from > output_object_block through to switch_section did not have the same > special casing for the vtable section that exists in > assemble_variable. That's some ugly code. You might consider factoring that code into a function and just calling it from both places. Your version doesn't seem to handle PECOFF, so I'd probably refactor from assemble_variable. > > However both these failures also occur on x86_64 - so I'm content to > declare victory on AArch64 as far as basic enablement goes. Cool. > > 1. Are the generic changes to varasm.c ok ? 2. Can we take the > AArch64 support in now, given this amount of testing ? Marcus / > Caroline ? 3. Any suggestions / helpful debug hints for VTV debugging > (other than turning VTV_DEBUG on and inspecting trace) ? I think that with refactoring they'd be good to go. No opinions on the AArch64 specific question -- call for the AArch64 maintainers. Good to see someone hacking on vtv. It's in my queue to look at as well. jeff
On 09/10/15 10:17, Ramana Radhakrishnan wrote: > This started as a Friday afternoon project ... > > It turned out enabling VTV for AArch64 and ARM was a matter of fixing PR67868 which essentially comes from building libvtv with section anchors turned on. The problem was that the flow of control from output_object_block through to switch_section did not have the same special casing for the vtable section that exists in assemble_variable. > > Once this was done, I managed to build and test aarch64-none-linux-gnu with --enable-vtable-verify, a similar test was done for armhf. > > Testing showed no regressions in the gcc/ g++ testsuites for aarch64 and armhf > Testing showed no failures in libvtv testsuite for aarch64 but a few more failures - see below. > Testing showed 2 failures in libstdc++-v3 testsuite compared to without vtable verification. > > FAIL: libstdc++-abi/abi_check > FAIL: experimental/filesystem/iterators/directory_iterator.cc execution test > > However both these failures also occur on x86_64 - so I'm content to declare victory on AArch64 > as far as basic enablement goes. > > On ARM I see the following failures that I still need to debug - I can see that the failure is because the write to _ZN4_VTVI1BE12__vtable_mapE does not elicit a SEGV but I need to go further than that. > > FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O0 -fvtable-verify=std execution test > FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O2 -fvtable-verify=std execution test > FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O0 -fvtable-verify=preinit execution test > FAIL: libvtv.cc/thunk_vtable_map_attack.cc -O2 -fvtable-verify=preinit execution test > > > Questions - > > 1. Are the generic changes to varasm.c ok ? > 2. Can we take the AArch64 support in now, given this amount of testing ? Marcus / Caroline ? + aarch64*-*-linux*) + VTV_SUPPORTED=yes ;; Ramana, Go ahead an add the aarch64 enable once you have the pre-requisite varasm changes approved. Cheers /Marcus > 3. Any suggestions / helpful debug hints for VTV debugging (other than turning VTV_DEBUG on and inspecting trace) ? > > There's an arm*-*-* hunk there but I'm easy about applying that right now and figuring out the issues > over time. In case we don't fix it for 6.0 we can rip the support out before release. > > > Thanks, > Ramana > > P.S. (Yes, I'll provide a Changelog :) ) >
diff --git a/gcc/varasm.c b/gcc/varasm.c index 48c3662..c182cd4 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -2236,7 +2236,7 @@ assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED, code is removed, the variables end up in the same section with a single comdat name. - FIXME: resolve_unique_section needs to deal better with + FIXME: resolve_unique_section needs to deal better with decls with both DECL_SECTION_NAME and DECL_ONE_ONLY. Once that is fixed, this if-else statement can be replaced with a single call to "switch_to_section (sect)". */ @@ -7329,7 +7329,33 @@ output_object_block (struct object_block *block) /* Switch to the section and make sure that the first byte is suitably aligned. */ - switch_to_section (block->sect); + /* The following bit of code ensures that vtable_map + variables are not only in the comdat section, but that + each variable has its own unique comdat name. If this + code is removed, the variables end up in the same section + with a single comdat name. + + FIXME: resolve_unique_section needs to deal better with + decls with both DECL_SECTION_NAME and DECL_ONE_ONLY. Once + that is fixed, this if-else statement can be replaced with + a single call to "switch_to_section (sect)". */ + if (SECTION_STYLE (block->sect) == SECTION_NAMED + && block->sect->named.name + && (strcmp (block->sect->named.name, ".vtable_map_vars") == 0)) + { +#if defined (OBJECT_FORMAT_ELF) + targetm.asm_out.named_section (block->sect->named.name, + block->sect->named.common.flags + | SECTION_LINKONCE, + DECL_NAME (block->sect->named.decl)); + in_section = block->sect; +#else + switch_to_section (block->sect); +#endif + } + else + switch_to_section (block->sect); + assemble_align (block->alignment); /* Define the values of all anchors relative to the current section diff --git a/libvtv/configure.tgt b/libvtv/configure.tgt index e543371..adcff5c 100644 --- a/libvtv/configure.tgt +++ b/libvtv/configure.tgt @@ -37,6 +37,10 @@ case "${target}" in sparc*-*-linux*) ;; arm*-*-linux*) + VTV_SUPPORTED=yes + ;; + aarch64*-*-linux*) + VTV_SUPPORTED=yes ;; x86_64-*-darwin[1]* | i?86-*-darwin[1]*) ;;