From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x936.google.com (mail-ua1-x936.google.com [IPv6:2607:f8b0:4864:20::936]) by sourceware.org (Postfix) with ESMTPS id 10B3A387093E for ; Wed, 26 Jun 2024 09:10:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 10B3A387093E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gwmail.gwu.edu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gwmail.gwu.edu ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 10B3A387093E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::936 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1719393042; cv=none; b=pL5lfyPbgE4C7IuD761v2MHBJPcH5wc2HKWWXEiRbLq5a6EvfzzZfZpDo7GG/zAAjEpGXnDxy36pGiJgQxvSXCiyVFvJytv5tJ7wTy310OJ7InjlrfrE0k9Takb/QQ6ZI6XNWUPSM3VbtNazceMQxXhcWksAzkPH5pQDBWX91IE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1719393042; c=relaxed/simple; bh=OdhKaVO7Io9bpAO1njKFB8KEw00LbANEdu0SxEmg3ss=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=YCtBBJHFNoB+wjbCOcyEpW+O49oebUDnSP2SiK3oz378So/BTm82X10al/W150SRbeFsR9AYaLheAvrSZlQVsKYBl4vgcqFiZc83e/rbu4EhhKSXzFUjQKfKDI0sUhe7l6ARwGhnyj9j+eJM748Rvj38/oS5/QtOcTk4G7/an6U= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ua1-x936.google.com with SMTP id a1e0cc1a2514c-80f879181fcso886804241.0 for ; Wed, 26 Jun 2024 02:10:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gwmail.gwu.edu; s=google; t=1719393040; x=1719997840; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=NzXzShxzQXQJN5CI9DXFdlc1tYUovOPjI+LmLpJoRIo=; b=JpT/gIBdn7u5/qbEs64ZcNznlsVZ9Hn4i0odGkFjRk1WLW7Gh/aEtql/WW3jgscqPB XxXoZDt+mPrbZnWXgj8WfHUrofH90LRrs0xGkN5vYqSUFndQsGTFxbhSv8kBDkj2YJvo wYmk3GhXeBzN80DBNpd3XOiaZ788UHxEXyimtbhxkNdf0dguFDVSU+HQ6o2id/M8hY8S 50rZS0krTQK3uFntHLy0Mr4fvckozY0VIHV76Zd+2ptYGg8lWh/K+HZbDAwTnIhewUSt 4fX9L+6FP+G1U67hYiIQuL659e91ifxHPvfjz6h0+jJnixz3Bu+StORZMOtB/8sUtf3y cU6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719393040; x=1719997840; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=NzXzShxzQXQJN5CI9DXFdlc1tYUovOPjI+LmLpJoRIo=; b=NzOUOTpx+21vnENL2zcTkPCxerSY0ISZKjXgG1UkquF7ur4KrM58HPBQVo8w4rJSDu Lw+ByboS27g7j8kqTCq++VnGEBwpWLjAjqMgT+rZz9YT0T7VIvAuymMBUx0TOsrnrckF rbiOQP2VL5DQtXoAFgobEhjRSHRZpq3ZOJ2MoQKuV9bOdEgYzhpiPRPblaCxHbf7UDO4 HFbFCL7oR/i7iYlYebgs92PaE0kWrJVeQvr5MqblXeJzZAzLjcTmSORd6n8T/tsnVhDB mlX85tj8jLwU0v+AJBv1DS+z3+abHTOAGugITgGFxUyTP1leyEhr+tAhV99gOkIEyyBr TAqw== X-Gm-Message-State: AOJu0YwYpd69BO6MHUGgDaYaXDiCQQkY+vRfVAs0AH9gIA4NoehA5i2b ZyGg0UK6pHWI2esykXgQGexAwsppT4mVHik+sHbq87YEj1orFrK4UjhvOXeUuJYLq1/su60E9lX XCwynzhBdcdDpqvfKzyQuUh1tJF3hr/ppb3TQ X-Google-Smtp-Source: AGHT+IFDp21c+XrHWIREMn7CM/FlUpjOwAji5atTV4lgQyuWu721ykIM18f9UH8o72T8oiMpkNYCJNFR8urQ0jvw/Ss= X-Received: by 2002:a67:f807:0:b0:48f:46db:7a11 with SMTP id ada2fe7eead31-48f52947b56mr7211600137.6.1719393040097; Wed, 26 Jun 2024 02:10:40 -0700 (PDT) MIME-Version: 1.0 References: <20240517195144.3084069-1-dmalcolm@redhat.com> <7fcde9e48b715db746026c8c3b1e8502dc04080e.camel@redhat.com> In-Reply-To: From: Eric Gallager Date: Wed, 26 Jun 2024 05:10:28 -0400 Message-ID: Subject: Re: PING: Re: [PATCH] selftest: invoke "diff" when ASSERT_STREQ fails To: David Malcolm Cc: gcc-patches@gcc.gnu.org, Ian Lance Taylor Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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: On Wed, May 29, 2024 at 5:06=E2=80=AFPM David Malcolm = wrote: > > On Wed, 2024-05-29 at 16:35 -0400, Eric Gallager wrote: > > On Tue, May 28, 2024 at 1:21=E2=80=AFPM David Malcolm > > wrote: > > > > > > Ping. > > > > > > This patch has actually been *very* helpful to me when debugging > > > selftest failures involving ASSERT_STREQ. > > > > > > Thanks > > > Dave > > > > > > > Currently `diff` is only listed under the "Tools/packages necessary > > for modifying GCC" section of install/prerequisites.html: > > https://gcc.gnu.org/install/prerequisites.html > > If it's going to become a dependency for actually running GCC, too, > > it > > should get moved to be documented elsewhere, IMO. > > All this is selftest code, and is turned off in a release configuration > of GCC. The code path that invokes "diff" is when a selftest is > failing, which is immediately before a hard failure of the *build* of > GCC. So arguably this is just a build-time thing for people > packaging/hacking on GCC, and thus not a new dependency for end-usage. > Well to be clear, I'm just asking for a documentation update here, so if you want to use wording that reflects all of that, I think that'd be fine. It seems like a useful idea overall, so don't let my documentation request hold you up from proceeding with it. > BTW I'm a bit hazy on the details of how "pex" is meant to work, so > hopefully someone more knowledgable than me can comment on that aspect > of the patch. It seems to work though. > I'm not too clear on it either; maybe one of the libiberty maintainers can chime in? Or wait, looks like there's just the one currently (Ian); cc-ing... > Dave > > > > > > On Fri, 2024-05-17 at 15:51 -0400, David Malcolm wrote: > > > > Currently when ASSERT_STREQ or ASSERT_STREQ_AT fail we print > > > > both strings to stderr. However it can be hard to figure out > > > > the problem (e.g. for 1-character differences in long strings). > > > > > > > > Extend the output by writing out the strings to tempfiles and > > > > invoking "diff -up" on them when we have such a selftest failure, > > > > to (I hope) simplify debugging. > > > > > > > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > > > > > > > OK for trunk? > > > > > > > > gcc/ChangeLog: > > > > * selftest.cc (selftest::print_diff): New function. > > > > (selftest::assert_streq): Call it when we have non-equal > > > > non-null strings. > > > > > > > > Signed-off-by: David Malcolm > > > > --- > > > > gcc/selftest.cc | 28 ++++++++++++++++++++++++++-- > > > > 1 file changed, 26 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/gcc/selftest.cc b/gcc/selftest.cc > > > > index 6438d86a6aa0..f58c0631908e 100644 > > > > --- a/gcc/selftest.cc > > > > +++ b/gcc/selftest.cc > > > > @@ -63,6 +63,26 @@ fail_formatted (const location &loc, const > > > > char > > > > *fmt, ...) > > > > abort (); > > > > } > > > > > > > > +/* Invoke "diff" to print the difference between VAL1 and VAL2 > > > > + on stdout. */ > > > > + > > > > +static void > > > > +print_diff (const location &loc, const char *val1, const char > > > > *val2) > > > > +{ > > > > + temp_source_file tmpfile1 (loc, ".txt", val1); > > > > + temp_source_file tmpfile2 (loc, ".txt", val2); > > > > + const char *args[] =3D {"diff", > > > > + "-up", > > > > + tmpfile1.get_filename (), > > > > + tmpfile2.get_filename (), > > > > + NULL}; > > > > + int exit_status =3D 0; > > > > + int err =3D 0; > > > > + pex_one (PEX_SEARCH | PEX_LAST, > > > > + args[0], CONST_CAST (char **, args), > > > > + NULL, NULL, NULL, &exit_status, &err); > > > > +} > > > > + > > > > /* Implementation detail of ASSERT_STREQ. > > > > Compare val1 and val2 with strcmp. They ought > > > > to be non-NULL; fail gracefully if either or both are NULL. > > > > */ > > > > @@ -89,8 +109,12 @@ assert_streq (const location &loc, > > > > if (strcmp (val1, val2) =3D=3D 0) > > > > pass (loc, "ASSERT_STREQ"); > > > > else > > > > - fail_formatted (loc, "ASSERT_STREQ (%s, %s)\n > > > > val1=3D\"%s\"\n > > > > val2=3D\"%s\"\n", > > > > - desc_val1, desc_val2, val1, val2); > > > > + { > > > > + print_diff (loc, val1, val2); > > > > + fail_formatted > > > > + (loc, "ASSERT_STREQ (%s, %s)\n val1=3D\"%s\"\n > > > > val2=3D\"%s\"\n", > > > > + desc_val1, desc_val2, val1, val2); > > > > + } > > > > } > > > > } > > > > > > > > > >