From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30153 invoked by alias); 26 Aug 2012 18:21:51 -0000 Received: (qmail 30142 invoked by uid 22791); 26 Aug 2012 18:21:49 -0000 X-SWARE-Spam-Status: No, hits=-5.4 required=5.0 tests=AWL,BAYES_40,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 26 Aug 2012 18:21:30 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q7QILUro020276 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 26 Aug 2012 14:21:30 -0400 Received: from host2.jankratochvil.net (ovpn-116-33.ams2.redhat.com [10.36.116.33]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q7QHhZaS032731 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Sun, 26 Aug 2012 13:43:38 -0400 Date: Sun, 26 Aug 2012 18:21:00 -0000 From: Jan Kratochvil To: Siddhesh Poyarekar Cc: gdb-patches@sourceware.org Subject: Re: bitpos expansion patches summary Message-ID: <20120826174335.GA23917@host2.jankratochvil.net> References: <20120805005350.150e5b74@spoyarek> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120805005350.150e5b74@spoyarek> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-08/txt/msg00786.txt.bz2 Hi Siddhesh, another part of the review. Thanks, Jan SAFE: (hppa-tdep.c:740): FUNC(write_memory): (ULONGEST to ssize_t) [(type)->length] RSAFE: (hppa-tdep.c:766): FUNC(memcpy): (ULONGEST to size_t) [(type)->length] SAFE(Bound value): (hppa-tdep.c:867): VARINIT(len): (ULONGEST to int) [(type)->length] SAFE(Bound value): (hppa-tdep.c:889): VARINIT(len): (ULONGEST to int) [(type)->length] SAFE: (hppa-tdep.c:1026): FUNC(regcache_cooked_write_part): (LONGEST to int) [len] SAFE: (hppa-tdep.c:1062): FUNC(write_memory): (LONGEST to ssize_t) [len] SAFE: (hppa-tdep.c:1068): FUNC(regcache_cooked_write_part): (LONGEST to int) [((len) < (8) ? (len) : (8))] SAFE: (hppa-tdep.c:1126): VARINIT(part): (ULONGEST to int) [(type)->length % 4] SAFE: (hppa-tdep.c:1141): CMP: (ULONGEST to int) [b < (type)->length] SAFE: (hppa-tdep.c:1181): ASSIGN: (LONGEST to int) [ offset = 8 - len] SAFE: (hppa-tdep.c:1203): ASSIGN: (LONGEST to int) [ offset = 8 - len] SAFE: (hppa-tdep.c:1225): FUNC(regcache_cooked_read_part): (LONGEST to int) [((len) < (8) ? (len) : (8))] SAFE: (hppa-tdep.c:1237): FUNC(regcache_cooked_write_part): (LONGEST to int) [((len) < (8) ? (len) : (8))] WPREVERTED(Only ok_for_watchpoint needs LONGEST): (i386-nat.c:613): FUNC(i386_length_and_rw_bits): (LONGEST to int) [len] - To be re-checked with updated watchpoints patch. WPREVERTED(Likewise): (i386-nat.c:647): FUNC(i386_length_and_rw_bits): (LONGEST to int) [len] - To be re-checked with updated watchpoints patch. SAFE: (i386-tdep.c:2394): FUNC(write_memory): (LONGEST to ssize_t) [len] RSAFE: (i386-tdep.c:2470): FUNC(memset): (LONGEST to size_t) [len] RSAFE: (i386-tdep.c:2489): FUNC(memcpy): (LONGEST to size_t) [len] RSAFE: (i386-tdep.c:2496): FUNC(memcpy): (LONGEST to size_t) [len - low_size] SAFE: (i386-tdep.c:2556): FUNC(regcache_raw_write_part): (LONGEST to int) [len] SAFE: (i386-tdep.c:2561): FUNC(regcache_raw_write_part): (LONGEST to int) [len - low_size] FIXED(Expand len): (i386-tdep.c:2594): VARINIT(len): (ULONGEST to int) [(type)->length] SAFE: (i386-tdep.c:2659): FUNC(read_memory): (ULONGEST to ssize_t) [(type)->length] FIXED(Expand len): (i386-tdep.c:3046): VARINIT(len): (ULONGEST to int) [(type)->length] FIXED(Expand len): (i386-tdep.c:3079): VARINIT(len): (ULONGEST to int) [(type)->length] FIXED(Expand len): (i386-tdep.c:3115): VARINIT(len): (ULONGEST to int) [(type)->length] SAFE: (i387-tdep.c:381): FUNC(get_frame_register_bytes): (ULONGEST to int) [(type)->length] FIXED(Expand n): (ia64-tdep.c:3234): VARINIT(n): (ULONGEST to int) [(type)->length / (float_elt_type)->length] SAFE: (ia64-tdep.c:3265): VARINIT(reglen): (ULONGEST to int) [(register_type(gdbarch, (0 + 8)))->length] SAFE: (ia64-tdep.c:3267): VARINIT(m): (ULONGEST to int) [(type)->length % reglen] SAFE: (ia64-tdep.c:3299): VARINIT(n): (ULONGEST to int) [(type)->length / (float_elt_type)->length] - You did expand n in ia64_extract_return_value, it can be array of floats. I did not investigate what happens if the array of floats is too large and it does not fit into ia64 registers. But I find safe to just expand it. SAFE: (ia64-tdep.c:3315): VARINIT(reglen): (ULONGEST to int) [(register_type(gdbarch, (0 + 8)))->length] SAFE: (ia64-tdep.c:3317): VARINIT(m): (ULONGEST to int) [(type)->length % reglen] SAFE: (infcall.c:1029): FUNC(read_value_memory): (ULONGEST to size_t) [(values_type)->length] SAFE: (infcall.c:1046): FUNC(read_value_memory): (ULONGEST to size_t) [(values_type)->length] REVERTED(siginfo type should fit into size_t): (infrun.c:6677): FUNC(xmalloc): (ULONGEST to size_t) [len] - I agree. So why have you changed 'size_t->ULONGEST len = TYPE_LENGTH (type);' ? That change should be therefore reverted. SAFE: (iq2000-tdep.c:513): VARINIT(size): (LONGEST to int) [len % 4 ? : 4] SAFE: (iq2000-tdep.c:564): VARINIT(size): (LONGEST to int) [len % 4 ? : 4] SAFE: (iq2000-tdep.c:581): FUNC(read_memory): (ULONGEST to ssize_t) [(type)->length] SAFE: (iq2000-tdep.c:739): ASSIGN: (LONGEST to int) [ slacklen = (4 - (typelen % 4)) % 4] SAFE: (iq2000-tdep.c:741): FUNC(memcpy): (LONGEST to size_t) [typelen] SAFE: (iq2000-tdep.c:772): FUNC(write_memory): (LONGEST to ssize_t) [typelen] SAFE: (iq2000-tdep.c:782): FUNC(write_memory): (LONGEST to ssize_t) [typelen] SAFE: (jit.c:343): ASSIGN: (ULONGEST to int) [ ptr_size = (ptr_type)->length] SAFE: (jit.c:384): ASSIGN: (ULONGEST to int) [ ptr_size = (ptr_type)->length] SAFE: (jv-lang.c:482): ASSIGN: (LONGEST to CORE_ADDR) [ (((type)->main_type->flds_bnds.fields[i]).loc.physaddr) = (boffset)] SAFE: (jv-lang.c:611): RET: (ULONGEST to int) [(objtype)->length] SAFE: (linux-record.c:500): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg2)))->length] SAFE: (linux-record.c:637): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg1)))->length] SAFE: (linux-record.c:713): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg1)))->length] SAFE: (linux-record.c:813): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg5)))->length] SAFE: (linux-record.c:856): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg2)))->length] SAFE: (linux-record.c:887): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg2)))->length] SAFE: (linux-record.c:918): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg2)))->length] SAFE: (linux-record.c:944): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg2)))->length] SAFE: (linux-record.c:981): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg2)))->length] SAFE: (linux-record.c:1041): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg2)))->length] SAFE: (linux-record.c:1055): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg1)))->length] SAFE: (linux-record.c:1392): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg2)))->length] SAFE: (linux-record.c:1877): FUNC(phex_nz): (ULONGEST to int) [(gdbarch_register_type(get_regcache_arch(regcache), (tdep->arg2)))->length] SAFE: (linux-tdep.c:169): FUNC(append_composite_type_field_aligned): (ULONGEST to int) [(long_type)->length] FIXED(Expand i): (m2-lang.c:121): CMP: (ULONGEST to UINT) [i < length] FIXED(Expand rep1, reps): (m2-lang.c:139): CMP: (ULONGEST to UINT) [rep1 < length] FIXED(Expand rep1, reps): (m2-lang.c:185): CMP: (ULONGEST to UINT) [i < length] SAFE: (m2-typeprint.c:377): CMP: (size_t to ULONGEST) [(type)->length < sizeof(LONGEST)] SAFE: (m32c-tdep.c:415): VARINIT(containing_len): (ULONGEST to int) [(reg->rx->type)->length] SAFE: (m32c-tdep.c:418): VARINIT(elt_len): (ULONGEST to int) [(reg->type)->length] SAFE: (m32c-tdep.c:435): ASSIGN: (ULONGEST to int) [ elt_offset = (reg->rx->type)->length - elt_offset - elt_len] SAFE: (m32c-tdep.c:451): FUNC(memset): (ULONGEST to size_t) [(reg->type)->length] SAFE: (m32c-tdep.c:479): VARINIT(high_bytes): (ULONGEST to int) [(reg->rx->type)->length] SAFE: (m32c-tdep.c:480): VARINIT(low_bytes): (ULONGEST to int) [(reg->ry->type)->length] SAFE: (m32c-tdep.c:485): CMP: (int to ULONGEST) [(reg->type)->length == high_bytes + low_bytes] SAFE: (m32c-tdep.c:510): VARINIT(high_bytes): (ULONGEST to int) [(reg->rx->type)->length] SAFE: (m32c-tdep.c:511): VARINIT(low_bytes): (ULONGEST to int) [(reg->ry->type)->length] SAFE: (m32c-tdep.c:515): CMP: (int to ULONGEST) [(reg->type)->length == high_bytes + low_bytes] SAFE: (m32c-tdep.c:539): VARINIT(len): (ULONGEST to int) [(tdep->r0->type)->length] SAFE: (m32c-tdep.c:577): VARINIT(len): (ULONGEST to int) [(tdep->r0->type)->length] SAFE: (m32c-tdep.c:2074): VARINIT(ptr_len): (ULONGEST to int) [(tdep->ptr_voyd)->length] SAFE: (m32r-tdep.c:260): VARINIT(len): (ULONGEST to int) [(type)->length] SAFE: (m32r-tdep.c:744): FUNC(memcpy): (LONGEST to size_t) [len] SAFE: (m68hc11-tdep.c:1222): FUNC(write_memory): (LONGEST to ssize_t) [len] SAFE: (m68hc11-tdep.c:1275): FUNC(regcache_raw_write_part): (LONGEST to int) [2 - len] SAFE: (m68hc11-tdep.c:1275): FUNC(regcache_raw_write_part): (LONGEST to int) [len] SAFE: (m68hc11-tdep.c:1278): FUNC(regcache_raw_write_part): (LONGEST to int) [4 - len] SAFE: (m68hc11-tdep.c:1279): FUNC(regcache_raw_write_part): (LONGEST to int) [len - 2] FIXED(Expand len): (m68hc11-tdep.c:1294): VARINIT(len): (ULONGEST to int) [(type)->length] SAFE: (m68kbsd-tdep.c:47): VARINIT(fp_len): (ULONGEST to int) [(gdbarch_register_type(gdbarch, regnum))->length] FIXED(Expand param for get_frame_register_bytes): (m68k-tdep.c:225): FUNC(get_frame_register_bytes): (ULONGEST to int) [(type)->length] - I am not against expanding LEN of get_frame_register_bytes, it has no use but it is a good sanity check instead of checking all the callers. But in this case BTW it is not needed, only TYPE_CODE_FLT can be passed to get_frame_register_bytes, TYPE_CODE_FLT is always small enough. SAFE: (m68k-tdep.c:301): FUNC(memcpy): (LONGEST to size_t) [len] SAFE: (m68k-tdep.c:306): FUNC(memcpy): (LONGEST to size_t) [len - 4] SAFE: (m68k-tdep.c:344): FUNC(regcache_raw_write_part): (LONGEST to int) [4 - len] SAFE: (m68k-tdep.c:344): FUNC(regcache_raw_write_part): (LONGEST to int) [len] SAFE: (m68k-tdep.c:347): FUNC(regcache_raw_write_part): (LONGEST to int) [8 - len] SAFE: (m68k-tdep.c:348): FUNC(regcache_raw_write_part): (LONGEST to int) [len - 4] FIXED(Expand len): (m68k-tdep.c:389): VARINIT(len): (ULONGEST to int) [(type)->length] SAFE: (m68k-tdep.c:428): FUNC(read_memory): (ULONGEST to ssize_t) [(type)->length] SAFE: (m68k-tdep.c:468): FUNC(read_memory): (ULONGEST to ssize_t) [(type)->length] SAFE: (m68k-tdep.c:535): FUNC(write_memory): (LONGEST to ssize_t) [len] SAFE: (m88k-tdep.c:165): VARINIT(len): (ULONGEST to int) [(type)->length] SAFE: (m88k-tdep.c:192): VARINIT(len): (ULONGEST to int) [(type)->length] FIXED(Expand to size_t)|ENSURED_SIZET: (m88k-tdep.c:311): VARINIT(len): (ULONGEST to int) [(type)->length] - ENSURED_SIZET is redundant, there is above already: const bfd_byte *valbuf = value_contents (args[i]); FIXED(Expand to LONGEST): (m88k-tdep.c:312): VARINIT(stack_word): (LONGEST to int) [num_stack_words] REVERTED(num_register_words bound by number of registers): (m88k-tdep.c:316): VARINIT(register_word): (LONGEST to int) [num_register_words] SAFE: (m88k-tdep.c:404): FUNC(memcpy): (LONGEST to size_t) [len] SAFE: (m88k-tdep.c:410): FUNC(memcpy): (LONGEST to size_t) [len] SAFE: (m88k-tdep.c:426): FUNC(memcpy): (LONGEST to size_t) [len] UNRELATED: (mdebugread.c:1016): ASSIGN: (int to SHORT) [ (t)->main_type->nfields = nfields] - True, 'short nfields;' is not this bug. SAFE: (mdebugread.c:1035): CMP: (SHORT to ULONGEST) [(t)->length == (t)->main_type->nfields] SAFE: (mdebugread.c:1238): ASSIGN: (bfd_vma to LONGEST) [ ((*f).loc.bitpos) = (sh->value)] - Not so clear to me, --disable-64-bit-bfd cannot support >=2GB inferiors but it should support >256MB structs. Still when it also affects only STABS I do not think it needs a fix. SAFE: (microblaze-tdep.c:571): FUNC(memcpy): (ULONGEST to size_t) [(type)->length] SAFE: (microblaze-tdep.c:608): FUNC(memcpy): (LONGEST to size_t) [len]