Message ID | 1503595405-89600-1-git-send-email-weimin.pan@oracle.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 80587 invoked by alias); 24 Aug 2017 17:34:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 80166 invoked by uid 89); 24 Aug 2017 17:34:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_PASS, UNPARSEABLE_RELAY autolearn=ham version=3.3.2 spammy=4187, 4387 X-HELO: aserp1040.oracle.com Received: from aserp1040.oracle.com (HELO aserp1040.oracle.com) (141.146.126.69) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 24 Aug 2017 17:34:05 +0000 Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id v7OHY30f019366 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gdb-patches@sourceware.org>; Thu, 24 Aug 2017 17:34:03 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id v7OHY20e014016 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gdb-patches@sourceware.org>; Thu, 24 Aug 2017 17:34:03 GMT Received: from abhmp0014.oracle.com (abhmp0014.oracle.com [141.146.116.20]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id v7OHY22k020478 for <gdb-patches@sourceware.org>; Thu, 24 Aug 2017 17:34:02 GMT Received: from wmpan.us.oracle.com (/10.147.27.127) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 24 Aug 2017 10:34:02 -0700 From: Weimin Pan <weimin.pan@oracle.com> To: gdb-patches@sourceware.org Subject: [PATCH] break gdb build on 32-bit host with ADI support Date: Thu, 24 Aug 2017 12:23:25 -0500 Message-Id: <1503595405-89600-1-git-send-email-weimin.pan@oracle.com> |
Commit Message
Weimin Pan
Aug. 24, 2017, 5:23 p.m. UTC
The problem of failing to build with arm-linux-gnueabihf-g++-4.8 was that type CORE_ADDR is of "unsigned long" on a 64-bit machine but is of type "unsigned long long" on a 32 bit system, which caused type mismatch when passing arguments or printing errors. Fixed the problem in three places - (1) change type of "blksize" and "nbits" in struct adi_stat_t to "unsigned long long"; (2) explicitly cast argument 3 type when calling target_auxv_search(); (2) use %llx with explicit cast when printing type CORE_ADDR in either printf_filtered() or error(). --- gdb/sparc64-tdep.c | 21 ++++++++++++--------- 1 files changed, 12 insertions(+), 9 deletions(-)
Comments
On 08/24/2017 06:23 PM, Weimin Pan wrote: > diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c > index 6f4fca7..0da2ae5 100644 > --- a/gdb/sparc64-tdep.c > +++ b/gdb/sparc64-tdep.c > @@ -90,12 +90,12 @@ static struct cmd_list_element *sparc64adilist = NULL; > typedef struct > { > /* The ADI block size. */ > - unsigned long blksize; > + unsigned long long blksize; > > /* Number of bits used for an ADI version tag which can be > * used together with the shift value for an ADI version tag > * to encode or extract the ADI version value in a pointer. */ > - unsigned long nbits; > + unsigned long long nbits; Do you really need to count 64-bit bits? :-P :-) (Formatting of comment is incorrect for GNU code, BTW. No '*' on each line.) > > /* The maximum ADI version tag value supported. */ > int max_version; > @@ -223,9 +223,10 @@ adi_available (void) > > proc->stat.checked_avail = true; > if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, > - &proc->stat.blksize) <= 0) > + (CORE_ADDR *)&proc->stat.blksize) <= 0) Please don't introduce potential aliasing problems. Also, missing space before &. Either make blksize really be a CORE_ADDR or do CORE_ADDR value; if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, &value) <= 0) return false; proc->stat.blksize = value; > - target_auxv_search (¤t_target, AT_ADI_NBITS, &proc->stat.nbits); > + target_auxv_search (¤t_target, AT_ADI_NBITS, > + (CORE_ADDR *)&proc->stat.nbits); > proc->stat.max_version = (1 << proc->stat.nbits) - 2; > proc->stat.is_avail = true; > Ditto. > @@ -346,7 +347,8 @@ adi_read_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags) > if (!adi_is_addr_mapped (vaddr, size)) > { > adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); > - error(_("Address at 0x%lx is not in ADI maps"), vaddr*ast.blksize); > + error(_("Address at 0x%llx is not in ADI maps"), > + (long long)(vaddr*ast.blksize)); > } Use paddress instead? Also spaces around '*' and after the cast. > > int target_errno; > @@ -366,7 +368,8 @@ adi_write_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags) > if (!adi_is_addr_mapped (vaddr, size)) > { > adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); > - error(_("Address at 0x%lx is not in ADI maps"), vaddr*ast.blksize); > + error(_("Address at 0x%llx is not in ADI maps"), > + (long long)(vaddr*ast.blksize)); Ditto. > } > > int target_errno; > @@ -387,7 +390,7 @@ adi_print_versions (CORE_ADDR vaddr, size_t cnt, unsigned char *tags) > while (cnt > 0) > { > QUIT; > - printf_filtered ("0x%016lx:\t", vaddr * adi_stat.blksize); > + printf_filtered ("0x%016llx:\t", (long long)(vaddr*adi_stat.blksize)); paddress / hex_string / phex_nz ? > for (int i = maxelts; i > 0 && cnt > 0; i--, cnt--) > { > if (tags[v_idx] == 0xff) /* no version tag */ > @@ -418,7 +421,7 @@ do_examine (CORE_ADDR start, int bcnt) > if (read_cnt == -1) > error (_("No ADI information")); > else if (read_cnt < cnt) > - error(_("No ADI information at 0x%lx"), vaddr); > + error(_("No ADI information at 0x%llx"), (long long)vaddr); padress. > > adi_print_versions (vstart, cnt, buf); > > @@ -438,7 +441,7 @@ do_assign (CORE_ADDR start, size_t bcnt, int version) > if (set_cnt == -1) > error (_("No ADI information")); > else if (set_cnt < cnt) > - error(_("No ADI information at 0x%lx"), vaddr); > + error(_("No ADI information at 0x%llx"), (long long)vaddr); paddress. BTW, this cast to long here: static CORE_ADDR adi_normalize_address (CORE_ADDR addr) { adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); if (ast.nbits) return ((CORE_ADDR)(((long)addr << ast.nbits) >> ast.nbits)); return addr; } looks suspiciously bogus to me. Consider a 32-bit host remote/cross debugging a SPARC64 target machine. Also consider a Win64-hosted GDB. Thanks, Pedro Alves
On 8/24/2017 11:07 AM, Pedro Alves wrote: > On 08/24/2017 06:23 PM, Weimin Pan wrote: > >> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c >> index 6f4fca7..0da2ae5 100644 >> --- a/gdb/sparc64-tdep.c >> +++ b/gdb/sparc64-tdep.c >> @@ -90,12 +90,12 @@ static struct cmd_list_element *sparc64adilist = NULL; >> typedef struct >> { >> /* The ADI block size. */ >> - unsigned long blksize; >> + unsigned long long blksize; >> >> /* Number of bits used for an ADI version tag which can be >> * used together with the shift value for an ADI version tag >> * to encode or extract the ADI version value in a pointer. */ >> - unsigned long nbits; >> + unsigned long long nbits; > Do you really need to count 64-bit bits? :-P :-) Since the value of either nbits or blksize is between 0 and 64, no, I really don't. But without a 32-bit host, I'm simply play safe here so that the compiler won't bark. > (Formatting of comment is incorrect for GNU code, BTW. No '*' > on each line.) Corrected. >> >> /* The maximum ADI version tag value supported. */ >> int max_version; >> @@ -223,9 +223,10 @@ adi_available (void) >> >> proc->stat.checked_avail = true; >> if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, >> - &proc->stat.blksize) <= 0) >> + (CORE_ADDR *)&proc->stat.blksize) <= 0) > Please don't introduce potential aliasing problems. Also, missing > space before &. > > Either make blksize really be a CORE_ADDR or do > > CORE_ADDR value; > if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, &value) <= 0) > return false; > proc->stat.blksize = value; Since neither blksize nor nbits is a CORE_ADDR, I'm taking your second suggestion. >> - target_auxv_search (¤t_target, AT_ADI_NBITS, &proc->stat.nbits); >> + target_auxv_search (¤t_target, AT_ADI_NBITS, >> + (CORE_ADDR *)&proc->stat.nbits); >> proc->stat.max_version = (1 << proc->stat.nbits) - 2; >> proc->stat.is_avail = true; >> > Ditto. > >> @@ -346,7 +347,8 @@ adi_read_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags) >> if (!adi_is_addr_mapped (vaddr, size)) >> { >> adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); >> - error(_("Address at 0x%lx is not in ADI maps"), vaddr*ast.blksize); >> + error(_("Address at 0x%llx is not in ADI maps"), >> + (long long)(vaddr*ast.blksize)); >> } > Use paddress instead? Also spaces around '*' and after the cast. Where is paddress defined? I tried casting to "uint64" which yields to "unsigned long" on a 64-bit host and didn't bode well with %llx. >> >> int target_errno; >> @@ -366,7 +368,8 @@ adi_write_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags) >> if (!adi_is_addr_mapped (vaddr, size)) >> { >> adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); >> - error(_("Address at 0x%lx is not in ADI maps"), vaddr*ast.blksize); >> + error(_("Address at 0x%llx is not in ADI maps"), >> + (long long)(vaddr*ast.blksize)); > Ditto. > >> } >> >> int target_errno; >> @@ -387,7 +390,7 @@ adi_print_versions (CORE_ADDR vaddr, size_t cnt, unsigned char *tags) >> while (cnt > 0) >> { >> QUIT; >> - printf_filtered ("0x%016lx:\t", vaddr * adi_stat.blksize); >> + printf_filtered ("0x%016llx:\t", (long long)(vaddr*adi_stat.blksize)); > paddress / hex_string / phex_nz ? ?? >> for (int i = maxelts; i > 0 && cnt > 0; i--, cnt--) >> { >> if (tags[v_idx] == 0xff) /* no version tag */ >> @@ -418,7 +421,7 @@ do_examine (CORE_ADDR start, int bcnt) >> if (read_cnt == -1) >> error (_("No ADI information")); >> else if (read_cnt < cnt) >> - error(_("No ADI information at 0x%lx"), vaddr); >> + error(_("No ADI information at 0x%llx"), (long long)vaddr); > padress. > >> >> adi_print_versions (vstart, cnt, buf); >> >> @@ -438,7 +441,7 @@ do_assign (CORE_ADDR start, size_t bcnt, int version) >> if (set_cnt == -1) >> error (_("No ADI information")); >> else if (set_cnt < cnt) >> - error(_("No ADI information at 0x%lx"), vaddr); >> + error(_("No ADI information at 0x%llx"), (long long)vaddr); > paddress. > > BTW, this cast to long here: > > static CORE_ADDR > adi_normalize_address (CORE_ADDR addr) > { > adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); > > if (ast.nbits) > return ((CORE_ADDR)(((long)addr << ast.nbits) >> ast.nbits)); > return addr; > } > > looks suspiciously bogus to me. Consider a 32-bit host > remote/cross debugging a SPARC64 target machine. Also consider > a Win64-hosted GDB. Good point. Changing it to: return ((long long)(((long long)addr << ast.nbits) >> ast.nbits)); Thanks. > > Thanks, > Pedro Alves >
On 08/24/2017 08:09 PM, Wei-min Pan wrote: > > > On 8/24/2017 11:07 AM, Pedro Alves wrote: >> On 08/24/2017 06:23 PM, Weimin Pan wrote: >> >>> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c >>> index 6f4fca7..0da2ae5 100644 >>> --- a/gdb/sparc64-tdep.c >>> +++ b/gdb/sparc64-tdep.c >>> @@ -90,12 +90,12 @@ static struct cmd_list_element *sparc64adilist = >>> NULL; >>> typedef struct >>> { >>> /* The ADI block size. */ >>> - unsigned long blksize; >>> + unsigned long long blksize; >>> /* Number of bits used for an ADI version tag which can be >>> * used together with the shift value for an ADI version tag >>> * to encode or extract the ADI version value in a pointer. */ >>> - unsigned long nbits; >>> + unsigned long long nbits; >> Do you really need to count 64-bit bits? :-P :-) > > Since the value of either nbits or blksize is between 0 and 64, > no, I really don't. But without a 32-bit host, I'm simply play > safe here so that the compiler won't bark. >> (Formatting of comment is incorrect for GNU code, BTW. No '*' >> on each line.) > > Corrected. > >>> /* The maximum ADI version tag value supported. */ >>> int max_version; >>> @@ -223,9 +223,10 @@ adi_available (void) >>> proc->stat.checked_avail = true; >>> if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, >>> - &proc->stat.blksize) <= 0) >>> + (CORE_ADDR *)&proc->stat.blksize) <= 0) >> Please don't introduce potential aliasing problems. Also, missing >> space before &. >> >> Either make blksize really be a CORE_ADDR or do >> >> CORE_ADDR value; >> if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, &value) <= 0) >> return false; >> proc->stat.blksize = value; > > Since neither blksize nor nbits is a CORE_ADDR, I'm taking your second > suggestion. Then you don't need the - unsigned long nbits; + unsigned long long nbits; change anymore.. > >>> @@ -346,7 +347,8 @@ adi_read_versions (CORE_ADDR vaddr, size_t size, >>> unsigned char *tags) >>> if (!adi_is_addr_mapped (vaddr, size)) >>> { >>> adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); >>> - error(_("Address at 0x%lx is not in ADI maps"), >>> vaddr*ast.blksize); >>> + error(_("Address at 0x%llx is not in ADI maps"), >>> + (long long)(vaddr*ast.blksize)); >>> } >> Use paddress instead? Also spaces around '*' and after the cast. > > Where is paddress defined? I tried casting to "uint64" which yields to > "unsigned long" on a 64-bit host and didn't bode well with %llx. $ grep paddress *.h utils.h:extern const char *paddress (struct gdbarch *gdbarch, CORE_ADDR addr); >>> @@ -387,7 +390,7 @@ adi_print_versions (CORE_ADDR vaddr, size_t cnt, >>> unsigned char *tags) >>> while (cnt > 0) >>> { >>> QUIT; >>> - printf_filtered ("0x%016lx:\t", vaddr * adi_stat.blksize); >>> + printf_filtered ("0x%016llx:\t", (long >>> long)(vaddr*adi_stat.blksize)); >> paddress / hex_string / phex_nz ? > > ?? Try grepping for those things. >> static CORE_ADDR >> adi_normalize_address (CORE_ADDR addr) >> { >> adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); >> >> if (ast.nbits) >> return ((CORE_ADDR)(((long)addr << ast.nbits) >> ast.nbits)); >> return addr; >> } >> >> looks suspiciously bogus to me. Consider a 32-bit host >> remote/cross debugging a SPARC64 target machine. Also consider >> a Win64-hosted GDB. > > Good point. Changing it to: > > return ((long long)(((long long)addr << ast.nbits) >> ast.nbits)); > > Thanks. Still looks odd to me. Why are you shifting signed types, for instance? Any why do you need the casts in the first place, BTW? Thanks, Pedro Alves
On 8/24/2017 12:23 PM, Pedro Alves wrote: > On 08/24/2017 08:09 PM, Wei-min Pan wrote: >> >> On 8/24/2017 11:07 AM, Pedro Alves wrote: >>> On 08/24/2017 06:23 PM, Weimin Pan wrote: >>> >>>> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c >>>> index 6f4fca7..0da2ae5 100644 >>>> --- a/gdb/sparc64-tdep.c >>>> +++ b/gdb/sparc64-tdep.c >>>> @@ -90,12 +90,12 @@ static struct cmd_list_element *sparc64adilist = >>>> NULL; >>>> typedef struct >>>> { >>>> /* The ADI block size. */ >>>> - unsigned long blksize; >>>> + unsigned long long blksize; >>>> /* Number of bits used for an ADI version tag which can be >>>> * used together with the shift value for an ADI version tag >>>> * to encode or extract the ADI version value in a pointer. */ >>>> - unsigned long nbits; >>>> + unsigned long long nbits; >>> Do you really need to count 64-bit bits? :-P :-) >> Since the value of either nbits or blksize is between 0 and 64, >> no, I really don't. But without a 32-bit host, I'm simply play >> safe here so that the compiler won't bark. >>> (Formatting of comment is incorrect for GNU code, BTW. No '*' >>> on each line.) >> Corrected. >> >>>> /* The maximum ADI version tag value supported. */ >>>> int max_version; >>>> @@ -223,9 +223,10 @@ adi_available (void) >>>> proc->stat.checked_avail = true; >>>> if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, >>>> - &proc->stat.blksize) <= 0) >>>> + (CORE_ADDR *)&proc->stat.blksize) <= 0) >>> Please don't introduce potential aliasing problems. Also, missing >>> space before &. >>> >>> Either make blksize really be a CORE_ADDR or do >>> >>> CORE_ADDR value; >>> if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, &value) <= 0) >>> return false; >>> proc->stat.blksize = value; >> Since neither blksize nor nbits is a CORE_ADDR, I'm taking your second >> suggestion. > Then you don't need the > > - unsigned long nbits; > + unsigned long long nbits; > > change anymore.. > >>>> @@ -346,7 +347,8 @@ adi_read_versions (CORE_ADDR vaddr, size_t size, >>>> unsigned char *tags) >>>> if (!adi_is_addr_mapped (vaddr, size)) >>>> { >>>> adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); >>>> - error(_("Address at 0x%lx is not in ADI maps"), >>>> vaddr*ast.blksize); >>>> + error(_("Address at 0x%llx is not in ADI maps"), >>>> + (long long)(vaddr*ast.blksize)); >>>> } >>> Use paddress instead? Also spaces around '*' and after the cast. >> Where is paddress defined? I tried casting to "uint64" which yields to >> "unsigned long" on a 64-bit host and didn't bode well with %llx. > $ grep paddress *.h > utils.h:extern const char *paddress (struct gdbarch *gdbarch, CORE_ADDR addr); On a 64-bit host: ... sparc64-tdep.c: In function 'void do_assign(CORE_ADDR, size_t, int)': sparc64-tdep.c:449:56: error: expected ')' before 'vaddr' error(_("No ADI information at 0x%llx"), (paddress)vaddr); ^~~~~ sparc64-tdep.c:449:61: error: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type 'const char* (*)(gdbarch*, CORE_ADDR) {ak a const char* (*)(gdbarch*, long unsigned int)}' [-Werror=format=] error(_("No ADI information at 0x%llx"), (paddress)vaddr); ^ cc1plus: all warnings being treated as errors make: *** [sparc64-tdep.o] Error 1 >>>> @@ -387,7 +390,7 @@ adi_print_versions (CORE_ADDR vaddr, size_t cnt, >>>> unsigned char *tags) >>>> while (cnt > 0) >>>> { >>>> QUIT; >>>> - printf_filtered ("0x%016lx:\t", vaddr * adi_stat.blksize); >>>> + printf_filtered ("0x%016llx:\t", (long >>>> long)(vaddr*adi_stat.blksize)); >>> paddress / hex_string / phex_nz ? >> ?? > Try grepping for those things. > > >>> static CORE_ADDR >>> adi_normalize_address (CORE_ADDR addr) >>> { >>> adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); >>> >>> if (ast.nbits) >>> return ((CORE_ADDR)(((long)addr << ast.nbits) >> ast.nbits)); >>> return addr; >>> } >>> >>> looks suspiciously bogus to me. Consider a 32-bit host >>> remote/cross debugging a SPARC64 target machine. Also consider >>> a Win64-hosted GDB. >> Good point. Changing it to: >> >> return ((long long)(((long long)addr << ast.nbits) >> ast.nbits)); >> >> Thanks. > Still looks odd to me. > Why are you shifting signed types, for instance? > Any why do you need the casts in the first place, BTW? We need to sign extend to get a normalized address, based on the definitions in ADI space: ADI versioned address - a VA with ADI bits (63-60) set Normalized address - a VA with bit 59 sign extended into ADI bits Thanks. > > Thanks, > Pedro Alves
On 08/24/2017 09:27 PM, Wei-min Pan wrote: >>>> Use paddress instead? Also spaces around '*' and after the cast. >>> Where is paddress defined? I tried casting to "uint64" which yields to >>> "unsigned long" on a 64-bit host and didn't bode well with %llx. >> $ grep paddress *.h >> utils.h:extern const char *paddress (struct gdbarch *gdbarch, >> CORE_ADDR addr); > > On a 64-bit host: > ... > sparc64-tdep.c: In function 'void do_assign(CORE_ADDR, size_t, int)': > sparc64-tdep.c:449:56: error: expected ')' before 'vaddr' > error(_("No ADI information at 0x%llx"), (paddress)vaddr); > ^~~~~ > sparc64-tdep.c:449:61: error: format '%llx' expects argument of type > 'long long > unsigned int', but argument 2 has type 'const char* (*)(gdbarch*, > CORE_ADDR) {ak > a const char* (*)(gdbarch*, long unsigned int)}' [-Werror=format=] > error(_("No ADI information at 0x%llx"), (paddress)vaddr); > ^ > cc1plus: all warnings being treated as errors > make: *** [sparc64-tdep.o] Error 1 Of course. paddress is a function. See any of the other ~400 uses in the tree. Something like: error(_("No ADI information at %s"), paddress (gdbarch, vaddr)); >>>> looks suspiciously bogus to me. Consider a 32-bit host >>>> remote/cross debugging a SPARC64 target machine. Also consider >>>> a Win64-hosted GDB. >>> Good point. Changing it to: >>> >>> return ((long long)(((long long)addr << ast.nbits) >> >>> ast.nbits)); >>> >>> Thanks. >> Still looks odd to me. >> Why are you shifting signed types, for instance? >> Any why do you need the casts in the first place, BTW? > > We need to sign extend to get a normalized address, based on the > definitions in ADI space: > > ADI versioned address - a VA with ADI bits (63-60) set > Normalized address - a VA with bit 59 sign extended into ADI bits OK, but then please do it with valid/portable C/C++ code, with the usual masking and xoring. Left shifting signed integer values is undefined C++11. Right shifting a signed integer is implementation defined. Thanks, Pedro Alves
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c index 6f4fca7..0da2ae5 100644 --- a/gdb/sparc64-tdep.c +++ b/gdb/sparc64-tdep.c @@ -90,12 +90,12 @@ static struct cmd_list_element *sparc64adilist = NULL; typedef struct { /* The ADI block size. */ - unsigned long blksize; + unsigned long long blksize; /* Number of bits used for an ADI version tag which can be * used together with the shift value for an ADI version tag * to encode or extract the ADI version value in a pointer. */ - unsigned long nbits; + unsigned long long nbits; /* The maximum ADI version tag value supported. */ int max_version; @@ -223,9 +223,10 @@ adi_available (void) proc->stat.checked_avail = true; if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, - &proc->stat.blksize) <= 0) + (CORE_ADDR *)&proc->stat.blksize) <= 0) return false; - target_auxv_search (¤t_target, AT_ADI_NBITS, &proc->stat.nbits); + target_auxv_search (¤t_target, AT_ADI_NBITS, + (CORE_ADDR *)&proc->stat.nbits); proc->stat.max_version = (1 << proc->stat.nbits) - 2; proc->stat.is_avail = true; @@ -346,7 +347,8 @@ adi_read_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags) if (!adi_is_addr_mapped (vaddr, size)) { adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); - error(_("Address at 0x%lx is not in ADI maps"), vaddr*ast.blksize); + error(_("Address at 0x%llx is not in ADI maps"), + (long long)(vaddr*ast.blksize)); } int target_errno; @@ -366,7 +368,8 @@ adi_write_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags) if (!adi_is_addr_mapped (vaddr, size)) { adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid)); - error(_("Address at 0x%lx is not in ADI maps"), vaddr*ast.blksize); + error(_("Address at 0x%llx is not in ADI maps"), + (long long)(vaddr*ast.blksize)); } int target_errno; @@ -387,7 +390,7 @@ adi_print_versions (CORE_ADDR vaddr, size_t cnt, unsigned char *tags) while (cnt > 0) { QUIT; - printf_filtered ("0x%016lx:\t", vaddr * adi_stat.blksize); + printf_filtered ("0x%016llx:\t", (long long)(vaddr*adi_stat.blksize)); for (int i = maxelts; i > 0 && cnt > 0; i--, cnt--) { if (tags[v_idx] == 0xff) /* no version tag */ @@ -418,7 +421,7 @@ do_examine (CORE_ADDR start, int bcnt) if (read_cnt == -1) error (_("No ADI information")); else if (read_cnt < cnt) - error(_("No ADI information at 0x%lx"), vaddr); + error(_("No ADI information at 0x%llx"), (long long)vaddr); adi_print_versions (vstart, cnt, buf); @@ -438,7 +441,7 @@ do_assign (CORE_ADDR start, size_t bcnt, int version) if (set_cnt == -1) error (_("No ADI information")); else if (set_cnt < cnt) - error(_("No ADI information at 0x%lx"), vaddr); + error(_("No ADI information at 0x%llx"), (long long)vaddr); }