From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id cWxNF33RjWkkvDcAWB0awg (envelope-from ) for ; Thu, 12 Feb 2026 08:11:25 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=HUDIkzmy; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 5A26C1E089; Thu, 12 Feb 2026 08:11:25 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 0A1B81E089 for ; Thu, 12 Feb 2026 08:11:23 -0500 (EST) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 7650F4BA23C7 for ; Thu, 12 Feb 2026 13:11:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7650F4BA23C7 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=HUDIkzmy Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id E911B4BA2E14 for ; Thu, 12 Feb 2026 13:10:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E911B4BA2E14 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E911B4BA2E14 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1770901852; cv=none; b=jioqwQuGcgEQ3CQeTnFsKO36dgp4PQ4hK2BrXRHIQIy53ZzIO348gsJow8JHbPqj2QFIFlcM49/KNRSTJKLSRDNCc41K7Cvx1Ni8jWLzwQRLht39dIIZUnAkFuMrJj2OyfPfrAP7DGJHfb7HWtU7r+jH7SGuI9bcmxMlYOFLaOg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1770901852; c=relaxed/simple; bh=Su/ctj2HTdHdPNfJ+e07RRJbIkZb/4y2xwP0Axqp1Gs=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=YdoDdreZ5m98oH3bFQRHAnIjYrFexsKB6JtULA5te1jVEyld935HqXuC2MEHxs/pBVb5fYdSvSpIb/hibv5uEGHLt7YK2ggQi1vNoOVAB/0aiSTJZ/0Un93x/YQjA+4GZDuA+8iPZWm23GhTFoT2nvcmTs08QP0dLSVEcaYUXho= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E911B4BA2E14 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1770901851; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=a50ls7MO7sy6x8fKP0LEo7gGJ/xDVYGfMlZF6Ul/K7o=; b=HUDIkzmypdFL6SxcEqjXwz8dFlPFvD/Mqe1mV1yeBCogR+fYikp8rlX/O27fl4qJQgwjsS ux9Fi+3d+EQ9LZ1xAzfDR+Kg7Mai3Hd9t7DDYW13PdmmNWxVw+GLHEbmwUfupqIQ6FTsld AL6pKdsG3ani57rlCwbF7WBIUkfYE2Y= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-477-ChwSCxPyP5KySI37wgV5Cw-1; Thu, 12 Feb 2026 08:10:50 -0500 X-MC-Unique: ChwSCxPyP5KySI37wgV5Cw-1 X-Mimecast-MFC-AGG-ID: ChwSCxPyP5KySI37wgV5Cw_1770901849 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-47ee432070aso62248685e9.1 for ; Thu, 12 Feb 2026 05:10:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770901849; x=1771506649; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=osy0xwL2qV8u1URE1TTo6RvS70WGN+wOJsW5JsUQrzE=; b=HLamAqL1KWYMauD5swrVXLq42gJt8Wd018vuR6fZBN7qM2p90CUureTFb72VmwPqq2 AQahsw1YjCRBLbvlpG5q9YmTgRt+ptxMnVvLexzUpw46PzuS+odbvMjtqgl8ifh15qri 4pGPB90ax8OkPeBHEIrNorK1wbq4Q/gOdW73r7Ho1Pv1vHgIjBMGd19n2IXDu6ANFlYC wZoYqV074mCCBuiIoYhKqzcApoR/sBbudGpJO16c/XW+B7P9MrT4IJFKK9LciVhgf1CO ld5kaI5VRK3GLWnd+deYmZh2O+uS4GTfnpxpgrQDPGOXw5w67oi3ZrUsFXSgM6QFIyYp sCew== X-Gm-Message-State: AOJu0YwYa72hq3hqA4JoRsPKwergbpMmwMdLhjQaYS7dgyyBHi8u3276 H5yDrx1uRJMPMtSo4xaOtlTSx98Kn78n1j6Ww/uLVyuDLils3c8Rx2hHG/VgQqzcuuJBXjKfKlS 9/1ND1l1mQZtPupEcrbcMxvXobba7UpVMABbwmpGQf/R3++FUHKe+cfHiAuyBziz4d60wIuc= X-Gm-Gg: AZuq6aK2U8bM8v9xEkpuWJHpL5LnGeutMNUN4i2/tlLBHJKo79YmgHFDZ0Vn4E+JVA/ DNGdyW1N5gNdwr8Z+OdDHjWTE97oJjeN1LhDRO8QosTXzRPB+0O6O2YuETuTzZQRFfBCi4bFtHk fwfTnH3OO/rxE6dT9Q2L2e9jqFt04BR+V5mRuPAWE+/Hv+42CVnbuwPUtf8QXhgLHmclJLGyYYn 0srCxL8x/cPCD3DTWKc3XHjB/99Ppr4Oca6blPV7nlGYCLAXr7ZepQsa1I+pfIHorVonjVpRQ1k OFbHS6CgRpxl3cBC+lrTMsdNbASmRxv09XFOFS77dRMx4rXJZxupa8WMTbhz0DYGIbPgf3Q0f7Q OZ9eejlbKgLV7ifNG X-Received: by 2002:a05:600c:4fd2:b0:483:56c4:73ac with SMTP id 5b1f17b1804b1-483656ae387mr42108575e9.7.1770901848813; Thu, 12 Feb 2026 05:10:48 -0800 (PST) X-Received: by 2002:a05:600c:4fd2:b0:483:56c4:73ac with SMTP id 5b1f17b1804b1-483656ae387mr42108065e9.7.1770901848315; Thu, 12 Feb 2026 05:10:48 -0800 (PST) Received: from localhost ([31.111.84.232]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4834d82a4c4sm346549245e9.10.2026.02.12.05.10.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Feb 2026 05:10:47 -0800 (PST) From: Andrew Burgess To: "Rohr, Stephan" Cc: "gdb-patches@sourceware.org" , =?utf-8?Q?S?= =?utf-8?Q?=C3=A9bastien?= Darche , "simark@simark.ca" Subject: RE: [PATCH] gdb: ensure bp_location::section is set correct to avoid an assert In-Reply-To: References: <7febb0c1-7bbd-45d5-8ebe-91c34bb4a6ce@efficios.com> <87tt0qe7qf.fsf@redhat.com> <87ldm2dxcl.fsf@redhat.com> <6c31b667-db2d-453e-9597-9fe011c4766e@efficios.com> <87jz186xhv.fsf@redhat.com> <87o6m4ekar.fsf@redhat.com> Date: Thu, 12 Feb 2026 13:10:46 +0000 Message-ID: <87ldgyayzt.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: euHutvbpDTZD_i2QLcGyjLQTra3anPgZMcC1CSHatUw_1770901849 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org "Rohr, Stephan" writes: >> -----Original Message----- >> From: Andrew Burgess >> Sent: Wednesday, 4 February 2026 14:00 >> To: Rohr, Stephan >> Cc: gdb-patches@sourceware.org; S=C3=A9bastien Darche ; >> simark@simark.ca >> Subject: RE: [PATCH] gdb: ensure bp_location::section is set correct to = avoid an >> assert >>=20 >> "Rohr, Stephan" writes: >>=20 >> >> -----Original Message----- >> >> From: Andrew Burgess >> >> Sent: Monday, 6 October 2025 14:11 >> >> To: S=C3=A9bastien Darche ; simark@simark.ca >> >> Cc: gdb-patches@sourceware.org >> >> Subject: Re: [PATCH] gdb: ensure bp_location::section is set correct = to avoid >> an >> >> assert >> >> >> >> S=C3=A9bastien Darche writes: >> >> >> >> > On 9/25/25 17:40, Andrew Burgess wrote: >> >> >> Maybe the answer is as simple as moving the .section assignment in= to >> the >> >> >> earlier if block, something like: >> >> >> >> >> >> if (is_function && want_start_sal) >> >> >> { >> >> >> sal =3D find_function_start_sal (func_addr, NULL, self->fun= firstline); >> >> >> >> >> >> /* This breakpoint is for the ifunc case, FUNC_ADDR is can = be >> >> >> anywhere, in a completely different section to MSYMBOL, = or even >> >> >> in a different objfile! >> >> >> >> >> >> TODO: I haven't checked, maybe find_function_start_sal a= lready >> >> >> fills this stuff in for us? Or maybe it could be made t= oo? >> >> >> For now I'm assuming all we have is an address, but this= needs >> >> >> checking. */ >> >> >> sal.section =3D find_pc_overlay (func_addr); >> >> >> if (sal.section =3D=3D nullptr) >> >> >> sal.section =3D find_pc_section (func_addr); >> >> >> } >> >> >> else >> >> >> { >> >> >> sal.objfile =3D objfile; >> >> >> sal.msymbol =3D msymbol; >> >> >> /* Store func_addr, not the minsym's address in case this w= as an >> >> >> =09 ifunc that hasn't been resolved yet. */ >> >> >> if (is_function) >> >> >> =09sal.pc =3D func_addr; >> >> >> else >> >> >> =09sal.pc =3D msymbol->value_address (objfile); >> >> >> sal.pspace =3D current_program_space; >> >> >> >> >> >> /* We can assign the section based on MSYMBOL here because = the >> >> >> breakpoint is actually being placed at (or near) MSYMBOL= . */ >> >> >> sal.section =3D msymbol->obj_section (objfile); >> >> >> } >> >> >> >> >> > >> >> > To answer your question on whether find_function_start_sal does fil= l >> >> > this for us : it depends. It manages to do it on amd64 but not on a= mdgpu. >> >> > >> >> > By default, the sal does not contain a valid section. It's only whe= n we >> >> > try to adjust the pc past the prologue (skip_prologue_sal) that a >> >> > section is computed for the pc at the start of the function. If we = do >> >> > have a prologue, then we assign that section (symtab.c:3914). If no= t >> >> > (and that is the case on amdgpu), then we're left with an empty >> >> > sal.section. I would say the behavior is not really consistent. >> >> > >> >> > I would agree it could be made to. >> >> >> Does this look like a valid path forward maybe? >> >> > >> >> > Your solution seems to work for the gnu-ifunc test and fixes the >> >> > regression for gdb.rocm/displaced-stepping.exp - so I'd say it's a = good >> >> > aproach. I am not familiar with overlays, so I can't really judge i= f the >> >> > change would impact how they are handled. >> >> > >> >> > I think it would be best to ensure find_function_start_sal has a >> >> > consistent behavior across architectures. I'll submit a small patch >> >> > which should address this. This would also at least reduce the chan= ce >> >> > for another bug like this to appear somewhere else : >> >> > >> >> >> >> Hi, >> >> >> >> I'm proposing the patch below. You should double check that this sti= ll >> >> addresses the issue you're seeing with the amdgpu target. Given Simo= n's >> >> concerns, I do wonder if there might still be some issues with this >> >> related to overlay debugging, but without any way to test it, and no >> >> known overlay users, I think we can probably just ignore that for now= . >> >> >> >> If this fixes your regression, then maybe we should merge this, and >> >> figure any other issues out later? >> >> >> >> Thanks, >> >> Andrew >> >> >> >> --- >> >> >> >> commit 6ffea587445eeacf8b2962de6d3b00d6efa98213 >> >> Author: Andrew Burgess >> >> Date: Mon Oct 6 10:27:08 2025 +0100 >> >> >> >> gdb: fixes for setting the section in minsym_found >> >> >> >> After this commit: >> >> >> >> commit 6f7ad2381ae72aa592ada4a0921265aa3292b1fa >> >> Date: Wed Sep 3 19:57:42 2025 +0100 >> >> >> >> gdb: ensure bp_location::section is set correct to avoid an= assert >> >> >> >> Some issues were reported as a result of the bp_location::section >> >> being left as NULL by the call to find_pc_overlay that was >> >> introduced. See this thread: >> >> >> >> https://inbox.sourceware.org/gdb-patches/7febb0c1-7bbd-45d5- >> 8ebe- >> >> 91c34bb4a6ce@efficios.com >> >> >> >> The problem was that code_breakpoint::add_location relies on the >> >> section being set in order to deduce the gdbarch. If the section= is >> >> not set then the gdbarch is deduced using the breakpoint's gdbarc= h. >> >> >> >> The bug was reported by the ROCm engineers, who have inferiors >> running >> >> mixed host and GPU code, and so rely on the section being set in = order >> >> to establish the correct architecture for a specific address. >> >> >> >> During discussion in the above thread Simon pointed out that the >> >> change made in the above commit might not be correct anyway for >> >> overlay debugging (does that even work, or is it used any more?),= as >> >> the commit relies on establishing a section by calling >> >> find_pc_overlay. However, when presented with multiple possible >> >> sections, find_pc_overlay cannot know which section to select, an= d so >> >> just picks one. This could be different from the section of the >> >> minimal_symbol we already had to hand. >> >> >> >> This patch I think should (at least) resolve the issues the ROCm >> >> engineers are seeing. >> >> >> >> Instead of always calling find_pc_overlay I have moved the sectio= n >> >> assignment inside the if/then/else blocks with the following >> >> reasoning. >> >> >> >> In the 'else' block, this is the non-function or non-ifunc case, = the >> >> address used is based on the msymbol's address, and so should be = in >> >> the same section. In this case we can use the msymbol's section. >> >> >> >> In the 'if' block things are more complicated. This could be the >> >> ifunc case, in which case func_addr could have been adjusted to a >> >> different section, or even different objfile. >> >> >> >> Further, when we call find_function_start_sal, we pass in just an >> >> address, so the SAL being returned isn't going to consider which >> >> overlay section the original msymbol was from, which could cause >> >> problems for overlay debugging maybe? >> >> >> >> Anyway, I'm ignoring that for now, as fixing that would be a whol= e big >> >> thing. So I'm proposing that, if find_function_start_sal returns= a >> >> symtab_and_line with a section set, then we use that section. >> >> Otherwise, we can try to figure out a section. >> >> >> >> diff --git a/gdb/linespec.c b/gdb/linespec.c >> >> index 2ddc495babf..4d9c5ac26f3 100644 >> >> --- a/gdb/linespec.c >> >> +++ b/gdb/linespec.c >> >> @@ -4083,13 +4083,16 @@ minsym_found (struct linespec_state *self, >> >> struct objfile *objfile, >> >> >> >> CORE_ADDR func_addr; >> >> bool is_function =3D msymbol_is_function (objfile, msymbol, &func_= addr); >> >> + bool is_ifunc =3D false; >> >> >> >> if (is_function) >> >> { >> >> const char *msym_name =3D msymbol->linkage_name (); >> >> >> >> - if (msymbol->type () =3D=3D mst_text_gnu_ifunc >> >> -=09 || msymbol->type () =3D=3D mst_data_gnu_ifunc) >> >> + is_ifunc =3D (msymbol->type () =3D=3D mst_text_gnu_ifunc >> >> +=09=09 || msymbol->type () =3D=3D mst_data_gnu_ifunc); >> >> + >> >> + if (is_ifunc) >> >> =09want_start_sal =3D gnu_ifunc_resolve_name (msym_name, >> >> &func_addr); >> >> else >> >> =09want_start_sal =3D true; >> >> @@ -4098,7 +4101,32 @@ minsym_found (struct linespec_state *self, >> struct >> >> objfile *objfile, >> >> symtab_and_line sal; >> >> >> >> if (is_function && want_start_sal) >> >> - sal =3D find_function_start_sal (func_addr, NULL, self->funfirst= line); >> >> + { >> >> + sal =3D find_function_start_sal (func_addr, NULL, self->funfir= stline); >> >> + >> >> + /* If SAL already has a section then we'll use that. If not, = then we >> >> +=09 can try to find a section. >> >> + >> >> +=09 In the ifunc case though we cannot rely on the section of MSYMBO= L, >> >> +=09 the ifunc target could be in a different section, or even a >> >> +=09 different objfile, from the original MSYMBOL. For this case, we >> >> +=09 fall back to looking up a section based on FUNC_ADDR. >> >> + >> >> +=09 For the non-ifunc case, we can use the section of MSYMBOL, as >> >> +=09 that's how we filled in FUNC_ADDR, so they should be in the same >> >> +=09 section. */ >> >> + if (sal.section =3D=3D nullptr) >> >> +=09{ >> >> +=09 if (!is_ifunc) >> >> +=09 sal.section =3D msymbol->obj_section (objfile); >> >> +=09 else >> >> +=09 { >> >> +=09 sal.section =3D find_pc_overlay (func_addr); >> >> +=09 if (sal.section =3D=3D nullptr) >> >> +=09=09sal.section =3D find_pc_section (func_addr); >> >> +=09 } >> >> +=09} >> >> + } >> >> else >> >> { >> >> sal.objfile =3D objfile; >> >> @@ -4110,14 +4138,13 @@ minsym_found (struct linespec_state *self, >> >> struct objfile *objfile, >> >> else >> >> =09sal.pc =3D msymbol->value_address (objfile); >> >> sal.pspace =3D current_program_space; >> >> - } >> >> >> >> - /* Don't use the section from the msymbol, the code above might ha= ve >> >> - adjusted FUNC_ADDR, in which case the msymbol's section might n= ot >> be >> >> - the section containing FUNC_ADDR. It might not even be in the = same >> >> - objfile. As the section is primarily to assist with overlay >> >> - debugging, it should reflect the SAL's pc value. */ >> >> - sal.section =3D find_pc_overlay (sal.pc); >> >> + /* We can assign the section based on MSYMBOL here because the >> >> +=09 breakpoint is actually being placed at (or near) MSYMBOL. Note, >> >> +=09 this is not a path where ifunc resolution can have occurred, whi= ch >> >> +=09 could adjust FUNC_ADDR significantly. */ >> >> + sal.section =3D msymbol->obj_section (objfile); >> >> + } >> >> >> >> if (self->maybe_add_address (objfile->pspace (), sal.pc)) >> >> add_sal_to_sals (self, result, &sal, msymbol->natural_name (), f= alse); >> > >> > Hi all, >> > >> > I came across the same issue when debugging a remote inferior that is >> compiled w/o debug symbols and a >> > breakpoint is inserted based on the function name that is called in th= e >> inferior. >> > >> > Our test started to regress with the introduction of patch >> > "gdb: ensure bp_location::section is set correct to avoid an assert". = As >> mentioned in the patch above, >> > GDB uses the default gdbarch to insert the breakpoint if the section i= s NULL. >> This causes issues later. >> > >> > I applied the patch; our breakpoint insertion issue is fixed with this= patch. I >> reviewed the patch, it is >> > reasonable to me. Only thing I wonder is the usage of 'find_pc_ovelay= '; this >> always returns NULL if >> > overlay debugging is not used? But I guess that's fixed by using >> 'find_pc_section' in this case. >>=20 >> Hey Stephan, >>=20 >> Could you confirm if you were testing against master, or against a >> release branch? If you're using a release branch, could you check if >> the issue is still present on master. >>=20 >> I'm surprised that commit 539fc2164f44a doesn't fix the issue you're >> seeing, but this is only available on master right now. >>=20 >> If you are testing on master, and you still applied my patch then you >> must have resolved the merge conflicts, could you post your diff so I >> can see how you resolved things. >>=20 >> Thanks, >> Andrew > > > Hi Andrew, > > I was testing against the GDB 17.1 release branch, with Intel GPU specifi= c patches > on top. > > I'm not able to test against master as I would need to cherry-pick those > patches first. > > I applied your patch on our internal branch before I tried the patch seri= es from S=C3=A9bastien, > which is on master but not on the GDB 17.1 release. The patch you origin= ally submitted > fixed the issue. > > Next, I cherry-picked the commits from S=C3=A9bastien's patch series w/o = applying your patch, > which also works in my case. With these applied, your patch doesn't appl= y cleanly > anymore in the "else" branch. > > I wonder if we still need the "else" part as we're now obtaining the sect= ion based on > the "msymbol" at the beginning of the function and asserting it is not NU= LL in the > else branch. I think the assert is fine, at least for ELF input. I believe that every ELF symbol is assigned to a section, if the symbol isn't associated with a section in the ELF then I believe either BFD or GDB assigns it to the absolute section as a fall back. At least, that's my understanding. Now it's possible some none-ELF file type does things differently, in which case, I guess this assert could be a problem then. But even if the assert did trigger, it's not clear if the right solution would be to change the assert to handle the NULL case, or to change how msymbols are parsed such that every symbol has a section. Until I have an actual example before me that triggers the assert, I'm inclined to leave it be for now. Thanks, Andrew