From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id E06963858D1E for ; Fri, 20 Jan 2023 13:03:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E06963858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674219794; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=R5tjLB9KH6odi1oGt6mviqEVe2oScjIcuCypAALPDF0=; b=O4lFYitJUsA79QZ/9l4PvyLquzb3B20EMDhD3I+Mzrws1rOxL2inIIF6m/QXNbfY13BiPV 0CAtZyQijVlkaslAoWcX6MaLYTuMFkzXf0bSLcM/R0qwzvl1tAc1bxaoV5367E+uCtp66/ nFkH4A1Ngy6Sm4AeKGlp1AgVaqC9Qv0= Received: from mail-il1-f197.google.com (mail-il1-f197.google.com [209.85.166.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-587-vJFKooCdMhqZrzYmNIGJDg-1; Fri, 20 Jan 2023 08:03:13 -0500 X-MC-Unique: vJFKooCdMhqZrzYmNIGJDg-1 Received: by mail-il1-f197.google.com with SMTP id i14-20020a056e020d8e00b003034b93bd07so3663897ilj.14 for ; Fri, 20 Jan 2023 05:03:13 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=R5tjLB9KH6odi1oGt6mviqEVe2oScjIcuCypAALPDF0=; b=Bp9LcyToYDTF0+bZcXFKj1v/8qNAAxlu8Uz6WTf8jxQCWh2/j3wiLps+LX5z4JFYB6 VQ8QYY2KrOLpbRq8MUMy7nDREsMgWSSt5xfsmm9nznQNQ076/n1Ar2vgxEirVbnDu+Mh S8nSY8507RWNFk9cmsoz1CjGPuDQ1SBxdffwcIpcZ7EQ/SzvSkSdOkS/eOxOEvXGY8BK b/dTUzxyBSCUSiy/qjx20+8BBzussy+9pb8XKbnx4VLUKyg/EB1fZZJ/TY38Tuoim0m7 UpRgGYJNYzgXClGeM7loHSN/KgTTiWvOkD++7++jGu13rqv4yN3Or9MXkFMR8ncVbEJH ee9g== X-Gm-Message-State: AFqh2ko1UOVtiaFUphkXDoPS2Pct/RvEyC2KJIrPpPn1E1oQEPBv3E7q YUKLjgusC3jWv6PKQZeHxZ296+lVVG8Bsi8aXWAHQ2r3P9P3n5Jq5M+E4pvP3HUeHs0jotRs3jv LihxWDkI6c+kwa3eNeIwqSQvB/TQRTkiX1bGzfeEMuMghZo/H+pys4dlYDnxEpR2hVrGIqNqubw == X-Received: by 2002:a05:6e02:1252:b0:30e:f631:79a8 with SMTP id j18-20020a056e02125200b0030ef63179a8mr11325666ilq.4.1674219792544; Fri, 20 Jan 2023 05:03:12 -0800 (PST) X-Google-Smtp-Source: AMrXdXuQtHOUrFX5q3RAhFW37rsU4Rye16tvdW0v37rBo0dxdrpYiwKTF9kdnYOE4aHhbR/CCXCBjw== X-Received: by 2002:a05:6e02:1252:b0:30e:f631:79a8 with SMTP id j18-20020a056e02125200b0030ef63179a8mr11325637ilq.4.1674219792037; Fri, 20 Jan 2023 05:03:12 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id w22-20020a05620a425600b006cbc00db595sm26195262qko.23.2023.01.20.05.03.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Jan 2023 05:03:11 -0800 (PST) From: Andrew Burgess To: Simon Marchi via Gdb-patches , gdb-patches@sourceware.org Cc: Simon Marchi Subject: Re: [PATCH 2/2] gdb/dwarf: fix UBsan crash in read_subrange_type In-Reply-To: <20230120050824.306976-2-simon.marchi@efficios.com> References: <20230120050824.306976-1-simon.marchi@efficios.com> <20230120050824.306976-2-simon.marchi@efficios.com> Date: Fri, 20 Jan 2023 13:03:09 +0000 Message-ID: <87fsc5e4tu.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Simon Marchi via Gdb-patches writes: > When running gdb.ada/arrayptr.exp (and others) on Ubuntu 22.04, with the > `gnat-11` package installed (not `gnat`), with UBSan activated, I get: > > (gdb) break foo.adb:40 > /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:17689:20: runtime error: shift exponent 127 is too large for 64-bit type 'long unsigned int' > > The problematic DIEs are: > > 0x00001460: DW_TAG_subrange_type > DW_AT_lower_bound [DW_FORM_data1] (0x00) > DW_AT_upper_bound [DW_FORM_data16] (ffffffffffffffff3f00000000000000) > DW_AT_name [DW_FORM_strp] ("foo__packed_array___XP7___XDLU_0__1180591620717411303423") > DW_AT_type [DW_FORM_ref4] (0x0000153f "long_long_long_unsigned") > DW_AT_GNAT_descriptive_type [DW_FORM_ref4] (0x0000147e) > DW_AT_artificial [DW_FORM_flag_present] (true) > > 0x0000153f: DW_TAG_base_type > DW_AT_byte_size [DW_FORM_data1] (0x10) > DW_AT_encoding [DW_FORM_data1] (DW_ATE_unsigned) > DW_AT_name [DW_FORM_strp] ("long_long_long_unsigned") > DW_AT_artificial [DW_FORM_flag_present] (true) > > When processed by this code: > > negative_mask = > -((ULONGEST) 1 << (base_type->length () * TARGET_CHAR_BIT - 1)); > if (low.kind () == PROP_CONST > && !base_type->is_unsigned () && (low.const_val () & negative_mask)) > low.set_const_val (low.const_val () | negative_mask); > > When the base type's length (16 bytes in this case) is larger than a > ULONGEST (typically 8 bytes), the bit shift is too large. > > My obvious fix is just to skip the fixup for base types larger than a > ULONGEST (8 bytes). I don't think we really handle constant attribute > values larger than 8 bytes anyway, so this is part of a much larger > problem. > > Add a test that replicates this situation, but uses bounds that fit in a > signed 64 bit, so we get a sensible result. > > Change-Id: I8d0a24f3edd83b44e0761a0ce38922d3e2e112fb > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29386 > --- > gdb/dwarf2/read.c | 29 ++++++++++++++++++--------- > gdb/testsuite/gdb.dwarf2/subrange.exp | 22 ++++++++++++++++++++ > 2 files changed, 41 insertions(+), 10 deletions(-) > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index 44b54f77de9..87846788604 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -17588,7 +17588,6 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu) > int low_default_is_valid; > int high_bound_is_count = 0; > const char *name; > - ULONGEST negative_mask; > > orig_base_type = read_subrange_index_type (die, cu); > > @@ -17684,15 +17683,25 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu) > with GCC, for instance, where the ambiguous DW_FORM_dataN form > is used instead. To work around that ambiguity, we treat > the bounds as signed, and thus sign-extend their values, when > - the base type is signed. */ > - negative_mask = > - -((ULONGEST) 1 << (base_type->length () * TARGET_CHAR_BIT - 1)); > - if (low.kind () == PROP_CONST > - && !base_type->is_unsigned () && (low.const_val () & negative_mask)) > - low.set_const_val (low.const_val () | negative_mask); > - if (high.kind () == PROP_CONST > - && !base_type->is_unsigned () && (high.const_val () & negative_mask)) > - high.set_const_val (high.const_val () | negative_mask); > + the base type is signed. > + > + Skip it if the base type's length is largest than ULONGEST, to avoid s/largest/larger/ > + the undefined behavior of a too large left shift. We don't really handle > + constants larger than 8 bytes anyway, at the moment. */ > + > + if (base_type->length () <= sizeof (ULONGEST)) > + { > + ULONGEST negative_mask > + = -((ULONGEST) 1 << (base_type->length () * TARGET_CHAR_BIT - 1)); > + > + if (low.kind () == PROP_CONST > + && !base_type->is_unsigned () && (low.const_val () & negative_mask)) > + low.set_const_val (low.const_val () | negative_mask); > + > + if (high.kind () == PROP_CONST > + && !base_type->is_unsigned () && (high.const_val () & negative_mask)) > + high.set_const_val (high.const_val () | negative_mask); > + } > > /* Check for bit and byte strides. */ > struct dynamic_prop byte_stride_prop; > diff --git a/gdb/testsuite/gdb.dwarf2/subrange.exp b/gdb/testsuite/gdb.dwarf2/subrange.exp > index 8a8443f31a8..556422629a3 100644 > --- a/gdb/testsuite/gdb.dwarf2/subrange.exp > +++ b/gdb/testsuite/gdb.dwarf2/subrange.exp > @@ -77,6 +77,26 @@ Dwarf::assemble $asm_file { > {name subrange_with_buggy_negative_bounds_variable} > {type :$subrange_with_buggy_negative_bounds_label} > } > + > + # This subrange's base type is 16-bytes long (although the bounds fit in > + # signed 64-bit). This is to test the fix for PR 29386. > + declare_labels a_16_byte_integer_label a_16_byte_subrange_label > + > + a_16_byte_integer_label: base_type { > + {byte_size 16 udata} > + {encoding @DW_ATE_signed} > + } > + > + a_16_byte_subrange_label: subrange_type { > + {lower_bound -9223372036854775808 DW_FORM_sdata} > + {upper_bound 9223372036854775807 DW_FORM_sdata} > + {type :$a_16_byte_integer_label} > + } > + > + DW_TAG_variable { > + {name a_16_byte_subrange_variable} > + {type :$a_16_byte_subrange_label} > + } > } > } > } > @@ -92,3 +112,5 @@ gdb_test "ptype TByteArray" \ > "type = array \\\[0\\.\\.191\\\] of byte" > gdb_test "ptype subrange_with_buggy_negative_bounds_variable" \ > "type = -16..-12" > +gdb_test "ptype a_16_byte_subrange_variable" \ > + "type = -9223372036854775808..9223372036854775807" As before, I'd use "\\.\\." here. Looks good with those nits fixed. Thanks, Andrew > -- > 2.39.1