public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v4 0/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
@ 2022-04-30 12:16 Zied Guermazi
  2022-04-30 12:16 ` [PATCH 1/1] " Zied Guermazi
  0 siblings, 1 reply; 17+ messages in thread
From: Zied Guermazi @ 2022-04-30 12:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: Zied Guermazi

get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE

Zied Guermazi (1):
  get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE

 gdb/nat/linux-btrace.c | 43 ++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 12:16 [PATCH v4 0/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE Zied Guermazi
@ 2022-04-30 12:16 ` Zied Guermazi
  2022-04-30 12:29   ` Andreas Schwab
  2022-05-02 16:38   ` Tom Tromey
  0 siblings, 2 replies; 17+ messages in thread
From: Zied Guermazi @ 2022-04-30 12:16 UTC (permalink / raw)
  To: gdb-patches; +Cc: Zied Guermazi

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)

---
 gdb/nat/linux-btrace.c | 43 ++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
index b0d6dcd7cf1..d99ec8e01e3 100644
--- a/gdb/nat/linux-btrace.c
+++ b/gdb/nat/linux-btrace.c
@@ -445,6 +445,17 @@ diagnose_perf_event_open_fail ()
   error (_("Failed to start recording: %s"), safe_strerror (errno));
 }
 
+/* Returns System page size.  */
+
+static long
+system_page_size ()
+{
+  long page_size = sysconf (_SC_PAGESIZE);
+  if (page_size == -1)
+    perror_with_name (_("Failed to get system page size"));
+  return page_size;
+}
+
 /* Enable branch tracing in BTS format.  */
 
 static struct btrace_target_info *
@@ -486,9 +497,10 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
   if (fd.get () < 0)
     diagnose_perf_event_open_fail ();
 
+  long page_size = system_page_size ();
   /* Convert the requested size in bytes to pages (rounding up).  */
-  pages = ((size_t) conf->size / PAGE_SIZE
-	   + ((conf->size % PAGE_SIZE) == 0 ? 0 : 1));
+  pages = ((size_t) conf->size / page_size
+	   + ((conf->size % page_size) == 0 ? 0 : 1));
   /* We need at least one page.  */
   if (pages == 0)
     pages = 1;
@@ -507,17 +519,17 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
       size_t length;
       __u64 data_size;
 
-      data_size = (__u64) pages * PAGE_SIZE;
+      data_size = (__u64) pages * (__u64) page_size;
 
       /* Don't ask for more than we can represent in the configuration.  */
       if ((__u64) UINT_MAX < data_size)
 	continue;
 
       size = (size_t) data_size;
-      length = size + PAGE_SIZE;
+      length = size + (size_t) page_size;
 
       /* Check for overflows.  */
-      if ((__u64) length != data_size + PAGE_SIZE)
+      if ((__u64) length != data_size + (__u64) page_size)
 	continue;
 
       errno = 0;
@@ -532,7 +544,7 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
 
   struct perf_event_mmap_page *header = (struct perf_event_mmap_page *)
     data.get ();
-  data_offset = PAGE_SIZE;
+  data_offset = (__u64) page_size;
 
 #if defined (PERF_ATTR_SIZE_VER5)
   if (offsetof (struct perf_event_mmap_page, data_size) <= header->size)
@@ -630,8 +642,9 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
     diagnose_perf_event_open_fail ();
 
   /* Allocate the configuration page. */
-  scoped_mmap data (nullptr, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
-		    fd.get (), 0);
+  long page_size = system_page_size ();
+  scoped_mmap data (nullptr, (size_t) page_size, PROT_READ | PROT_WRITE,
+		     MAP_SHARED, fd.get (), 0);
   if (data.get () == MAP_FAILED)
     error (_("Failed to map trace user page: %s."), safe_strerror (errno));
 
@@ -641,8 +654,8 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
   header->aux_offset = header->data_offset + header->data_size;
 
   /* Convert the requested size in bytes to pages (rounding up).  */
-  pages = ((size_t) conf->size / PAGE_SIZE
-	   + ((conf->size % PAGE_SIZE) == 0 ? 0 : 1));
+  pages = ((size_t) conf->size / page_size
+	   + ((conf->size % page_size) == 0 ? 0 : 1));
   /* We need at least one page.  */
   if (pages == 0)
     pages = 1;
@@ -661,7 +674,7 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
       size_t length;
       __u64 data_size;
 
-      data_size = (__u64) pages * PAGE_SIZE;
+      data_size = (__u64) pages * (__u64) page_size;
 
       /* Don't ask for more than we can represent in the configuration.  */
       if ((__u64) UINT_MAX < data_size)
@@ -732,7 +745,8 @@ linux_enable_btrace (ptid_t ptid, const struct btrace_config *conf)
 static enum btrace_error
 linux_disable_bts (struct btrace_tinfo_bts *tinfo)
 {
-  munmap((void *) tinfo->header, tinfo->bts.size + PAGE_SIZE);
+  long page_size = system_page_size ();
+  munmap ((void *) tinfo->header, (size_t) (tinfo->bts.size + page_size));
   close (tinfo->file);
 
   return BTRACE_ERR_NONE;
@@ -743,8 +757,9 @@ linux_disable_bts (struct btrace_tinfo_bts *tinfo)
 static enum btrace_error
 linux_disable_pt (struct btrace_tinfo_pt *tinfo)
 {
-  munmap((void *) tinfo->pt.mem, tinfo->pt.size);
-  munmap((void *) tinfo->header, PAGE_SIZE);
+  long page_size = system_page_size ();
+  munmap ((void *) tinfo->pt.mem, tinfo->pt.size);
+  munmap ((void *) tinfo->header, (size_t) page_size);
   close (tinfo->file);
 
   return BTRACE_ERR_NONE;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 12:16 ` [PATCH 1/1] " Zied Guermazi
@ 2022-04-30 12:29   ` Andreas Schwab
  2022-04-30 12:32     ` Zied Guermazi
  2022-05-02 16:38   ` Tom Tromey
  1 sibling, 1 reply; 17+ messages in thread
From: Andreas Schwab @ 2022-04-30 12:29 UTC (permalink / raw)
  To: Zied Guermazi; +Cc: gdb-patches

On Apr 30 2022, Zied Guermazi wrote:

> PAGE_SIZE is not defined in all build configurations, e.g cross compiling
> for aarch64 GNU/Linux machine.

Why is that file compiled in the first place?  The only configuration
using it is native x86-linux.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 12:29   ` Andreas Schwab
@ 2022-04-30 12:32     ` Zied Guermazi
  2022-04-30 12:52       ` Andreas Schwab
  0 siblings, 1 reply; 17+ messages in thread
From: Zied Guermazi @ 2022-04-30 12:32 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: gdb-patches

hi Andreas,

this is part of the preparation for offering the execution record and 
replay functionality on ARM processors using CoreSight ETM traces. 
Markus Metzger recommended in his review brining this change in a 
separated patch.

Kind Regards

Zied Guermazi

On 30.04.22 14:29, Andreas Schwab wrote:
> On Apr 30 2022, Zied Guermazi wrote:
>
>> PAGE_SIZE is not defined in all build configurations, e.g cross compiling
>> for aarch64 GNU/Linux machine.
> Why is that file compiled in the first place?  The only configuration
> using it is native x86-linux.
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 12:32     ` Zied Guermazi
@ 2022-04-30 12:52       ` Andreas Schwab
  2022-04-30 12:58         ` Zied Guermazi
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Schwab @ 2022-04-30 12:52 UTC (permalink / raw)
  To: Zied Guermazi; +Cc: gdb-patches

Why is it relevant whether this is cross compiling?

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 12:52       ` Andreas Schwab
@ 2022-04-30 12:58         ` Zied Guermazi
  2022-04-30 13:03           ` Andreas Schwab
  0 siblings, 1 reply; 17+ messages in thread
From: Zied Guermazi @ 2022-04-30 12:58 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: gdb-patches

hi Andreas,

the issue was first observed when activating this code while compiling 
on an x86 for an arm target.

/Zied

On 30.04.22 14:52, Andreas Schwab wrote:
> Why is it relevant whether this is cross compiling?
>
-- 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 12:58         ` Zied Guermazi
@ 2022-04-30 13:03           ` Andreas Schwab
  2022-04-30 13:25             ` Zied Guermazi
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Schwab @ 2022-04-30 13:03 UTC (permalink / raw)
  To: Zied Guermazi; +Cc: gdb-patches

How is that different from native compilation?

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 13:03           ` Andreas Schwab
@ 2022-04-30 13:25             ` Zied Guermazi
  2022-04-30 13:33               ` Andreas Schwab
  0 siblings, 1 reply; 17+ messages in thread
From: Zied Guermazi @ 2022-04-30 13:25 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: gdb-patches

Hi Andreas,

PAGE_SIZE for target system was not defined, in other cases PAGE_SIZE of 
the host system was used etc... :).

There are also some systems where the constant PAGE_SIZE is not defined 
at all. This patch is about using the portable posix API instead.

here are some further readings

https://github.com/intel/libipt/issues/11

/Zied


On 30.04.22 15:03, Andreas Schwab wrote:
> How is that different from native compilation?
>
-- 

*Zied Guermazi*
founder

Trande GmbH
Leuschnerstraße 2
69469 Weinheim/Germany

Mobile: +491722645127
mailto:zied.guermazi@trande.de <mailto:zied.guermazi@trande.de>

*Trande GmbH*
Leuschnerstraße 2, D-69469 Weinheim; Telefon: +491722645127
Sitz der Gesellschaft: Weinheim- Registergericht: AG Mannheim HRB 736209 
- Geschäftsführung: Zied Guermazi

*Confidentiality Note*
This message is intended only for the use of the named recipient(s) and 
may contain confidential and/or privileged information. If you are not 
the intended recipient, please contact the sender and delete the 
message. Any unauthorized use of the information contained in this 
message is prohibited.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 13:25             ` Zied Guermazi
@ 2022-04-30 13:33               ` Andreas Schwab
  2022-05-02 16:46                 ` Pedro Alves
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Schwab @ 2022-04-30 13:33 UTC (permalink / raw)
  To: Zied Guermazi; +Cc: gdb-patches

On Apr 30 2022, Zied Guermazi wrote:

> PAGE_SIZE for target system was not defined, in other cases PAGE_SIZE of
> the host system was used etc... :).

This file is only used in a native configuration.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 12:16 ` [PATCH 1/1] " Zied Guermazi
  2022-04-30 12:29   ` Andreas Schwab
@ 2022-05-02 16:38   ` Tom Tromey
  2022-05-02 16:50     ` Pedro Alves
  1 sibling, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2022-05-02 16:38 UTC (permalink / raw)
  To: Zied Guermazi; +Cc: gdb-patches

>>>>> "Zied" == Zied Guermazi <zied.guermazi@trande.de> writes:

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

Thanks.  I think this is ok.

Tom

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-04-30 13:33               ` Andreas Schwab
@ 2022-05-02 16:46                 ` Pedro Alves
  2022-05-02 18:01                   ` Zied Guermazi
  0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2022-05-02 16:46 UTC (permalink / raw)
  To: Andreas Schwab, Zied Guermazi; +Cc: gdb-patches

On 2022-04-30 14:33, Andreas Schwab wrote:
> On Apr 30 2022, Zied Guermazi wrote:
> 
>> PAGE_SIZE for target system was not defined, in other cases PAGE_SIZE of
>> the host system was used etc... :).
> 
> This file is only used in a native configuration.
> 

Zied, when you said:

  "the issue was first observed when activating this code while compiling on an x86 for an arm target."

are you talking about:

  --build=x86_64-linux --host=aarch64-linux

or:
  --host=x86_64-linux --target=aarch64-linux

?

I agree with Andreas -- linux-btrace.c is only included in the build on x86 hosts, so why are you seeing
this?  Did you tweak gdb/configure.nat to include linux-btrace.c in other configurations?  Or gdb/configure.tgt?
What exactly did you do?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-05-02 16:38   ` Tom Tromey
@ 2022-05-02 16:50     ` Pedro Alves
  2022-05-02 17:04       ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2022-05-02 16:50 UTC (permalink / raw)
  To: Tom Tromey, Zied Guermazi; +Cc: gdb-patches

On 2022-05-02 17:38, Tom Tromey wrote:
>>>>>> "Zied" == Zied Guermazi <zied.guermazi@trande.de> writes:
> 
> Zied> PAGE_SIZE is not defined in all build configurations, e.g cross compiling
> Zied> for aarch64 GNU/Linux machine. This patch gets the value at runtime using 
> Zied> posix call sysconf (_SC_PAGESIZE)
> 
> Thanks.  I think this is ok.

Please don't merge it yet.  Andreas' question is quite pertinent -- this file in only built on
x86 hosts.  The premise of the patch doesn't seems strange offhand.  Is the Aarch64 port
going to start using linux-btrace.c?  That isn't clear.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-05-02 16:50     ` Pedro Alves
@ 2022-05-02 17:04       ` Tom Tromey
  2022-05-02 17:08         ` Pedro Alves
  0 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2022-05-02 17:04 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, Zied Guermazi, gdb-patches

Pedro> Please don't merge it yet.  Andreas' question is quite pertinent
Pedro> -- this file in only built on x86 hosts.  The premise of the
Pedro> patch doesn't seems strange offhand.  Is the Aarch64 port going
Pedro> to start using linux-btrace.c?  That isn't clear.

Based on the thread I assumed it was in preparation for such a merge.

I suppose I don't really know for sure, but on the other hand it also
seemed like this wasn't harmful.

Tom

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-05-02 17:04       ` Tom Tromey
@ 2022-05-02 17:08         ` Pedro Alves
  0 siblings, 0 replies; 17+ messages in thread
From: Pedro Alves @ 2022-05-02 17:08 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Zied Guermazi, gdb-patches

On 2022-05-02 18:04, Tom Tromey wrote:
> Pedro> Please don't merge it yet.  Andreas' question is quite pertinent
> Pedro> -- this file in only built on x86 hosts.  The premise of the
> Pedro> patch doesn't seems strange offhand.  Is the Aarch64 port going
> Pedro> to start using linux-btrace.c?  That isn't clear.
> 
> Based on the thread I assumed it was in preparation for such a merge.
> 
> I suppose I don't really know for sure, but on the other hand it also
> seemed like this wasn't harmful.

Yeah, I agree it doesn't seem harmful.  I'd just like to clarify the
motivation, and make sure we have the right info in the commit log.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-05-02 16:46                 ` Pedro Alves
@ 2022-05-02 18:01                   ` Zied Guermazi
  2022-05-02 18:55                     ` Pedro Alves
  0 siblings, 1 reply; 17+ messages in thread
From: Zied Guermazi @ 2022-05-02 18:01 UTC (permalink / raw)
  To: Pedro Alves, Andreas Schwab, Tom Tromey; +Cc: gdb-patches

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)

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.

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.


Kind Regards

Zied Guermazi


On 02.05.22 18:46, Pedro Alves wrote:
> On 2022-04-30 14:33, Andreas Schwab wrote:
>> On Apr 30 2022, Zied Guermazi wrote:
>>
>>> PAGE_SIZE for target system was not defined, in other cases PAGE_SIZE of
>>> the host system was used etc... :).
>> This file is only used in a native configuration.
>>
> Zied, when you said:
>
>    "the issue was first observed when activating this code while compiling on an x86 for an arm target."
>
> are you talking about:
>
>    --build=x86_64-linux --host=aarch64-linux
>
> or:
>    --host=x86_64-linux --target=aarch64-linux
>
> ?
>
> I agree with Andreas -- linux-btrace.c is only included in the build on x86 hosts, so why are you seeing
> this?  Did you tweak gdb/configure.nat to include linux-btrace.c in other configurations?  Or gdb/configure.tgt?
> What exactly did you do?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-05-02 18:01                   ` Zied Guermazi
@ 2022-05-02 18:55                     ` Pedro Alves
  2022-05-02 19:16                       ` Zied Guermazi
  0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2022-05-02 18:55 UTC (permalink / raw)
  To: Zied Guermazi, Andreas Schwab, Tom Tromey; +Cc: gdb-patches

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE
  2022-05-02 18:55                     ` Pedro Alves
@ 2022-05-02 19:16                       ` Zied Guermazi
  0 siblings, 0 replies; 17+ messages in thread
From: Zied Guermazi @ 2022-05-02 19:16 UTC (permalink / raw)
  To: Pedro Alves, Andreas Schwab, Tom Tromey; +Cc: gdb-patches

hi Pedro,

the issue was observed twice: once when building on x86 using

"../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 once when building natively on Debian on aarch64 by using

"../binutils-gdb/configure --disable-ld --disable-gold --disable-gas 
--disable-sim --disable-gprofng --with-arm-cs"


I will change the comment and re-submit the patch


Kind Regards

Zied Guermazi.

On 02.05.22 20:55, Pedro Alves wrote:
> 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 sethttps://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
-- 

*Zied Guermazi*
founder

Trande GmbH
Leuschnerstraße 2
69469 Weinheim/Germany

Mobile: +491722645127
mailto:zied.guermazi@trande.de <mailto:zied.guermazi@trande.de>

*Trande GmbH*
Leuschnerstraße 2, D-69469 Weinheim; Telefon: +491722645127
Sitz der Gesellschaft: Weinheim- Registergericht: AG Mannheim HRB 736209 
- Geschäftsführung: Zied Guermazi

*Confidentiality Note*
This message is intended only for the use of the named recipient(s) and 
may contain confidential and/or privileged information. If you are not 
the intended recipient, please contact the sender and delete the 
message. Any unauthorized use of the information contained in this 
message is prohibited.


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-05-02 19:16 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-30 12:16 [PATCH v4 0/1] get page size using sysconf (_SC_PAGESIZE) instead of PAGE_SIZE Zied Guermazi
2022-04-30 12:16 ` [PATCH 1/1] " Zied Guermazi
2022-04-30 12:29   ` Andreas Schwab
2022-04-30 12:32     ` Zied Guermazi
2022-04-30 12:52       ` Andreas Schwab
2022-04-30 12:58         ` Zied Guermazi
2022-04-30 13:03           ` Andreas Schwab
2022-04-30 13:25             ` Zied Guermazi
2022-04-30 13:33               ` Andreas Schwab
2022-05-02 16:46                 ` Pedro Alves
2022-05-02 18:01                   ` Zied Guermazi
2022-05-02 18:55                     ` Pedro Alves
2022-05-02 19:16                       ` Zied Guermazi
2022-05-02 16:38   ` Tom Tromey
2022-05-02 16:50     ` Pedro Alves
2022-05-02 17:04       ` Tom Tromey
2022-05-02 17:08         ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).