exec: Build page-vary-common.c with -fno-lto

In bbc17caf81, we used an alias attribute to allow target_page
to be declared const, and yet be initialized late.

This fails when using LTO with several versions of gcc.
The compiler looks through the alias and decides that the const
variable is statically initialized to zero, then propagates that
zero to many uses of the variable.

This can be avoided by compiling one object file with -fno-lto.
In this way, any initializer cannot be seen, and the constant
propagation does not occur.

Since we are certain to have this separate compilation unit, we
can drop the alias attribute as well.  We simply have differing
declarations for target_page in different compilation units.
Drop the use of init_target_page, and drop the configure detection
for CONFIG_ATTRIBUTE_ALIAS.

In order to change the compilation flags for a file with meson,
we must use a static_library.  This runs into specific_ss, where
we would need to create many static_library instances.

Fix this by splitting page-vary.c: the page-vary-common.c part is
compiled once as a static_library, while the page-vary.c part is
left in specific_ss in order to handle the target-specific value
of TARGET_PAGE_BITS_MIN.

Reported-by: Gavin Shan <gshan@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210321211534.2101231-1-richard.henderson@linaro.org>
[PMD: Fix typo in subject, split original patch in 3]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Gavin Shan <gshan@redhat.com>
Message-Id: <20210322112427.4045204-4-f4bug@amsat.org>
[rth: Update MAINTAINERS]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
This commit is contained in:
Richard Henderson 2021-03-22 12:24:26 +01:00
parent 27eb9d65ff
commit 44b99a6d5f
7 changed files with 84 additions and 96 deletions

View File

@ -118,6 +118,7 @@ S: Maintained
F: softmmu/cpus.c
F: cpus-common.c
F: page-vary.c
F: page-vary-common.c
F: accel/tcg/
F: accel/stubs/tcg-stub.c
F: util/cacheinfo.c

19
configure vendored
View File

@ -4889,21 +4889,6 @@ if test "$plugins" = "yes" &&
"for this purpose. You can't build with --static."
fi
########################################
# See if __attribute__((alias)) is supported.
# This false for Xcode 9, but has been remedied for Xcode 10.
# Unfortunately, travis uses Xcode 9 by default.
attralias=no
cat > $TMPC << EOF
int x = 1;
extern const int y __attribute__((alias("x")));
int main(void) { return 0; }
EOF
if compile_prog "" "" ; then
attralias=yes
fi
########################################
# check if getauxval is available.
@ -5935,10 +5920,6 @@ if test "$atomic64" = "yes" ; then
echo "CONFIG_ATOMIC64=y" >> $config_host_mak
fi
if test "$attralias" = "yes" ; then
echo "CONFIG_ATTRIBUTE_ALIAS=y" >> $config_host_mak
fi
if test "$getauxval" = "yes" ; then
echo "CONFIG_GETAUXVAL=y" >> $config_host_mak
fi

View File

@ -216,11 +216,7 @@ static inline void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val
#ifdef TARGET_PAGE_BITS_VARY
# include "exec/page-vary.h"
#if defined(CONFIG_ATTRIBUTE_ALIAS) || !defined(IN_EXEC_VARY)
extern const TargetPageBits target_page;
#else
extern TargetPageBits target_page;
#endif
#ifdef CONFIG_DEBUG_TCG
#define TARGET_PAGE_BITS ({ assert(target_page.decided); target_page.bits; })
#define TARGET_PAGE_MASK ({ assert(target_page.decided); \

View File

@ -26,4 +26,9 @@ typedef struct {
uint64_t mask;
} TargetPageBits;
#ifdef IN_PAGE_VARY
extern bool set_preferred_target_page_bits_common(int bits);
extern void finalize_target_page_bits_common(int min);
#endif
#endif /* EXEC_PAGE_VARY_H */

View File

@ -1944,6 +1944,24 @@ specific_ss.add(when: 'CONFIG_TCG', if_true: files(
))
specific_ss.add(when: 'CONFIG_TCG_INTERPRETER', if_true: files('tcg/tci.c'))
# Work around a gcc bug/misfeature wherein constant propagation looks
# through an alias:
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99696
# to guess that a const variable is always zero. Without lto, this is
# impossible, as the alias is restricted to page-vary-common.c. Indeed,
# without lto, not even the alias is required -- we simply use different
# declarations in different compilation units.
pagevary = files('page-vary-common.c')
if get_option('b_lto')
pagevary_flags = ['-fno-lto']
if get_option('cfi')
pagevary_flags += '-fno-sanitize=cfi-icall'
endif
pagevary = static_library('page-vary-common', sources: pagevary,
c_args: pagevary_flags)
pagevary = declare_dependency(link_with: pagevary)
endif
common_ss.add(pagevary)
specific_ss.add(files('page-vary.c'))
subdir('backends')

54
page-vary-common.c Normal file
View File

@ -0,0 +1,54 @@
/*
* Variable page size handling -- target independent part.
*
* Copyright (c) 2003 Fabrice Bellard
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
#define IN_PAGE_VARY 1
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "exec/page-vary.h"
/* WARNING: This file must *not* be complied with -flto. */
TargetPageBits target_page;
bool set_preferred_target_page_bits_common(int bits)
{
/*
* The target page size is the lowest common denominator for all
* the CPUs in the system, so we can only make it smaller, never
* larger. And we can't make it smaller once we've committed to
* a particular size.
*/
if (target_page.bits == 0 || target_page.bits > bits) {
if (target_page.decided) {
return false;
}
target_page.bits = bits;
}
return true;
}
void finalize_target_page_bits_common(int min)
{
if (target_page.bits == 0) {
target_page.bits = min;
}
target_page.mask = -1ull << target_page.bits;
target_page.decided = true;
}

View File

@ -17,92 +17,25 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
#define IN_PAGE_VARY 1
#include "qemu/osdep.h"
#include "qemu-common.h"
#define IN_EXEC_VARY 1
#include "exec/exec-all.h"
#ifdef TARGET_PAGE_BITS_VARY
# ifdef CONFIG_ATTRIBUTE_ALIAS
/*
* We want to declare the "target_page" variable as const, which tells
* the compiler that it can cache any value that it reads across calls.
* This avoids multiple assertions and multiple reads within any one user.
*
* This works because we finish initializing the data before we ever read
* from the "target_page" symbol.
*
* This also requires that we have a non-constant symbol by which we can
* perform the actual initialization, and which forces the data to be
* allocated within writable memory. Thus "init_target_page", and we use
* that symbol exclusively in the two functions that initialize this value.
*
* The "target_page" symbol is created as an alias of "init_target_page".
*/
static TargetPageBits init_target_page;
/*
* Note that this is *not* a redundant decl, this is the definition of
* the "target_page" symbol. The syntax for this definition requires
* the use of the extern keyword. This seems to be a GCC bug in
* either the syntax for the alias attribute or in -Wredundant-decls.
*
* See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91765
*/
# pragma GCC diagnostic push
# pragma GCC diagnostic ignored "-Wredundant-decls"
extern const TargetPageBits target_page
__attribute__((alias("init_target_page")));
# pragma GCC diagnostic pop
# else
/*
* When aliases are not supported then we force two different declarations,
* by way of suppressing the header declaration with IN_EXEC_VARY.
* We assume that on such an old compiler, LTO cannot be used, and so the
* compiler cannot not detect the mismatched declarations, and all is well.
*/
TargetPageBits target_page;
# define init_target_page target_page
# endif
#endif
bool set_preferred_target_page_bits(int bits)
{
/*
* The target page size is the lowest common denominator for all
* the CPUs in the system, so we can only make it smaller, never
* larger. And we can't make it smaller once we've committed to
* a particular size.
*/
#ifdef TARGET_PAGE_BITS_VARY
assert(bits >= TARGET_PAGE_BITS_MIN);
if (init_target_page.bits == 0 || init_target_page.bits > bits) {
if (init_target_page.decided) {
return false;
}
init_target_page.bits = bits;
}
#endif
return set_preferred_target_page_bits_common(bits);
#else
return true;
#endif
}
void finalize_target_page_bits(void)
{
#ifdef TARGET_PAGE_BITS_VARY
if (init_target_page.bits == 0) {
init_target_page.bits = TARGET_PAGE_BITS_MIN;
}
init_target_page.mask = (target_long)-1 << init_target_page.bits;
init_target_page.decided = true;
/*
* For the benefit of an -flto build, prevent the compiler from
* hoisting a read from target_page before we finish initializing.
*/
barrier();
finalize_target_page_bits_common(TARGET_PAGE_BITS_MIN);
#endif
}