Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE

[Pedro Alves <pedro@palves.net>]

On 2022-05-02 19:01, Zied Guermazi wrote:
> hi Pedro,
> 
> I think I can clarify the situation here by giving more insight on the context. A short explanation is needed.
> 
> In current build system linux-btrace.c is only included when the host is a Linux running of an i386 cpu. (see gdb/configure.nat).  so the point of Andreas is pertinent with no further explanations of additional planned changes.
> 
> This patch is a preparation for another patch set for adding btrace on arm processors using ARM Coresight ETM traces. (see the patch set https://pi.simark.ca/gdb-patches/20210531213307.275079-1-zied.guermazi@trande.de/#r ). Markus Metzger in his review recommended submitting this change in a separated patch.
> 
> In the patch set for adding btrace on arm,  there is a need to build linux-btrace.c for hosts linux/aarch64 and linux/arm. gdb/configure.nat at line 233 looks now as following:
> 
> "
> 
>  /   linux)//
> //    case ${gdb_host_cpu} in//
> //        aarch64)//
> //        #  Host: AArch64 based machine running GNU/Linux//
> //        NATDEPFILES="${NATDEPFILES} aarch64-nat.o aarch64-linux-nat.o \//
> //        aarch32-linux-nat.o nat/aarch64-hw-point.o \//
> //        nat/aarch64-linux-hw-point.o \//
> //        nat/aarch64-linux.o \//
> //        *nat/linux-btrace.o* \//
> //        nat/aarch64-sve-linux-ptrace.o \//
> //        nat/aarch64-mte-linux-ptrace.o"//
> //        ;;//
> //        arc)//
> //        # Host: ARC based machine running GNU/Linux//
> //        NATDEPFILES="${NATDEPFILES} arc-linux-nat.o"//
> //        ;;//
> //        arm)//
> //        # Host: ARM based machine running GNU/Linux//
> //        NATDEPFILES="${NATDEPFILES} arm-linux-nat.o \//
> //        aarch32-linux-nat.o *nat/linux-btrace.o*"//
> //        ;;/
> 
> "
> 
> which means that the file is now included in the build for linux/aarch64 and linux/arm
> 
> 
> I have observed this issue when the build machine was an intel x86,  host and target were both aarch64-linux (and also arm-linux)

OK, thanks, that clarifies things.  I wasn't aware of that patch series.  The build and target machines (in autoconf terms) are
really irrelevant here.  The only machine that matters here is the host one.

> 
> using following configuration
> 
> "../binutils-gdb/configure --host aarch64-linux-gnu --target=aarch64-linux-gnu --disable-ld --disable-gold --disable-gas --disable-sim --disable-gprofng --with-arm-cs "
> 
> and when building natively on Debian on aarch64 and Debian on arm.

Did you really run configure on one machine, and then typed "make" on a different machine?
Anyhow, doesn't matter.

> 
> I was getting following build error message
> 
> "
> /../../binutils-gdb/gdb/nat/linux-btrace.c:799:30: error: ‘PAGE_SIZE’ was not declared in this scope; did you mean ‘PTRACE_SEIZE’?//
> //  799 |   scoped_mmap data (nullptr, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,//
> //      |                              ^~~~~~~~~//
> //      |                              PTRACE_SEIZE/
> 
> "
> 
> The issue was fixed with this patch.

Right.

So in the commit log, instead of:

 "PAGE_SIZE is not defined in all build configurations, e.g cross compiling
 for aarch64 GNU/Linux machine. This patch gets the value at runtime using 
 posix call sysconf (_SC_PAGESIZE)"

don't mention cross compiling at all, as that just confuses things.

I'd suggest instead:

 "Currently, linux-btrace.c is only built when the host is x86 or x86_64 GNU/Linux.
 Later patches will add btrace support on ARM processors using ARM Coresight ETM traces.
 That ran into the fact that PAGE_SIZE is not defined on Aarch64.  This patch fixes that
 by getting the page size value at runtime using POSIX sysconf(_SC_PAGESIZE) instead."

Pedro Alves