From patchwork Fri Dec 1 15:53:11 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 24676 Received: (qmail 117565 invoked by alias); 1 Dec 2017 15:53:17 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 117555 invoked by uid 89); 1 Dec 2017 15:53:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, KB_WAM_FROM_NAME_SINGLEWORD, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=incorporated, Reporting X-HELO: mx1.redhat.com Subject: Re: [PATCH] support: Add TEST_COMPARE macro To: Paul Eggert , Siddhesh Poyarekar , GNU C Library Cc: Adhemerval Zanella References: <8d63a2dc-f9e9-acaa-5ac5-ac5c4fbd6c9f@redhat.com> <8c399cde-3093-0195-a89a-ca230540cffb@gotplt.org> <8339a519-935a-019c-4a8f-798e37f6f346@redhat.com> <5c5fbfc4-0077-22ad-1fd4-53388acaf1e7@cs.ucla.edu> <96692ad0-180c-6e7c-79c8-e5c96d79e34c@cs.ucla.edu> <7e965c74-8113-72b9-276c-baa031df007e@redhat.com> <24bc9987-5af7-6e8a-9b3d-aeae4a45f60a@cs.ucla.edu> <72d1c0c0-44b3-69f7-7630-0d47be285a30@cs.ucla.edu> <2b76ec51-8dac-2fc9-ba32-512fdfbec2ff@cs.ucla.edu> <17e4d3db-8a69-e50c-07b1-71e1e8739d9d@redhat.com> <742d0fab-b17e-a7f5-62df-d87510454aa4@cs.ucla.edu> From: Florian Weimer Message-ID: Date: Fri, 1 Dec 2017 16:53:11 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <742d0fab-b17e-a7f5-62df-d87510454aa4@cs.ucla.edu> On 11/27/2017 07:53 PM, Paul Eggert wrote: > On 11/27/2017 10:43 AM, Florian Weimer wrote: >> I don't understand. >> >> It is perfectly reasonable to warn for >> >>    ch == EOF >> >> (with ch of type char) > > Sure, but that's a different topic. I was writing about the topic at > hand, which is that C integer comparison sometimes disagrees with > mathematical comparison. When ch is of type char, (ch == EOF) always > returns the mathematically-correct answer on glibc platforms. None of > the proposed TEST_COMPARE patches would catch the char-vs-EOF problem, > because they're not designed to catch it: they're designed to catch the > C-vs-mathematical comparison problem. Yeah, we got side-tracked. I incorporated your suggestion about rejecting sign-altering promotions into the attached patch. It also prints hexadecimal values with the appropriate (type-dependent) width. Thanks, Florian Subject: [PATCH] support: Add TEST_COMPARE macro To: libc-alpha@sourceware.org 2017-12-01 Florian Weimer * support/check.h (TEST_COMPARE): Define. (support_test_compare_failure): Declare. * support/Makefile (libsupport-routines): Add support_test_compare_failure. (tests): Add tst-test_compare. * support /support_test_compare_failure.c: New file. * support/tst-test_compare.c: Likewise. diff --git a/support/Makefile b/support/Makefile index 384fecb65a..bb81825fc2 100644 --- a/support/Makefile +++ b/support/Makefile @@ -54,6 +54,7 @@ libsupport-routines = \ support_record_failure \ support_run_diff \ support_shared_allocate \ + support_test_compare_failure \ support_write_file_string \ support_test_main \ support_test_verify_impl \ @@ -143,6 +144,7 @@ tests = \ tst-support_capture_subprocess \ tst-support_format_dns_packet \ tst-support_record_failure \ + tst-test_compare \ tst-xreadlink \ ifeq ($(run-built-tests),yes) diff --git a/support/check.h b/support/check.h index bdcd12952a..57adf84579 100644 --- a/support/check.h +++ b/support/check.h @@ -86,6 +86,64 @@ void support_test_verify_exit_impl (int status, const char *file, int line, does not support reporting failures from a DSO. */ void support_record_failure (void); +/* Compare the two integers LEFT and RIGHT and report failure if they + are different. */ +#define TEST_COMPARE(left, right) \ + ({ \ + typedef __typeof__ (left) __left_type; \ + typedef __typeof__ (right) __right_type; \ + __left_type __left_value = (left); \ + __right_type __right_value = (right); \ + /* Prevent use with floating-point and boolean types. */ \ + _Static_assert ((__left_type) 0.1 == (__left_type) 0, \ + "left value has floating-point type"); \ + _Static_assert ((__right_type) 0.1 == (__right_type) 0, \ + "right value has floating-point type"); \ + /* Prevent accidental use with larger-than-long long types. */ \ + _Static_assert (sizeof (__left_value) <= sizeof (long long), \ + "left value fits into long long"); \ + _Static_assert (sizeof (__right_value) <= sizeof (long long), \ + "right value fits into long long"); \ + /* Make sure that type promotion does not alter the sign. */ \ + enum \ + { \ + __left_is_unsigned = (__left_type) -1 > 0, \ + __right_is_unsigned = (__right_type) -1 > 0, \ + __left_promotes_to_right = __left_is_unsigned \ + && sizeof (__left_value) < sizeof (__right_value), \ + __right_promotes_to_left = __right_is_unsigned \ + && sizeof (__right_value) < sizeof (__left_value) \ + }; \ + _Static_assert (__left_is_unsigned == __right_is_unsigned \ + || __left_promotes_to_right \ + || __right_promotes_to_left, \ + "integer promotion may alter sign of operands"); \ + /* Compare the value. */ \ + if (__left_value != __right_value) \ + /* Pass the sign for printing the correct value. */ \ + support_test_compare_failure \ + (__FILE__, __LINE__, \ + #left, __left_value, __left_value < 0, sizeof (__left_type), \ + #right, __right_value, __right_value < 0, sizeof (__right_type)); \ + }) + +/* Internal implementation of TEST_COMPARE. LEFT_NEGATIVE and + RIGHT_NEGATIVE are used to store the sign separately, so that both + unsigned long long and long long arguments fit into LEFT_VALUE and + RIGHT_VALUE, and the function can still print the original value. + LEFT_SIZE and RIGHT_SIZE specify the size of the argument in bytes, + for hexadecimal formatting. */ +void support_test_compare_failure (const char *file, int line, + const char *left_expr, + long long left_value, + int left_negative, + int left_size, + const char *right_expr, + long long right_value, + int right_negative, + int right_size); + + /* Internal function called by the test driver. */ int support_report_failure (int status) __attribute__ ((weak, warn_unused_result)); diff --git a/support/support_test_compare_failure.c b/support/support_test_compare_failure.c new file mode 100644 index 0000000000..ab55b9f51a --- /dev/null +++ b/support/support_test_compare_failure.c @@ -0,0 +1,51 @@ +/* Reporting a numeric comparison failure. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include + +static void +report (const char *which, const char *expr, long long value, int negative, + int size) +{ + printf (" %s: ", which); + if (negative) + printf ("%lld", value); + else + printf ("%llu", (unsigned long long) value); + unsigned long long mask + = (~0ULL) >> (8 * (sizeof (unsigned long long) - size)); + printf (" (0x%llx); from: %s\n", (unsigned long long) value & mask, expr); +} + +void +support_test_compare_failure (const char *file, int line, + const char *left_expr, + long long left_value, + int left_negative, + int left_size, + const char *right_expr, + long long right_value, + int right_negative, + int right_size) +{ + support_record_failure (); + printf ("%s:%d: numeric comparison failure\n", file, line); + report (" left", left_expr, left_value, left_negative, left_size); + report ("right", right_expr, right_value, right_negative, right_size); +} diff --git a/support/tst-test_compare.c b/support/tst-test_compare.c new file mode 100644 index 0000000000..2356e800dc --- /dev/null +++ b/support/tst-test_compare.c @@ -0,0 +1,68 @@ +/* Basic test for the TEST_COMPARE macro. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include + +static void +subprocess (void *closure) +{ + char ch = 1; + /* These tests should fail. */ + TEST_COMPARE (ch, -1); /* Line 28. */ + TEST_COMPARE (2LL, -2LL); /* Line 29. */ + TEST_COMPARE (3L, (short) -3); /* Line 30. */ +} + +static int +do_test (void) +{ + /* This should succeed. */ + TEST_COMPARE (1, 1); + TEST_COMPARE (2LL, 2U); + + struct support_capture_subprocess proc = support_capture_subprocess + (&subprocess, NULL); + + /* Discard the reported error. */ + support_record_failure_reset (); + + puts ("info: *** subprocess output starts ***"); + fputs (proc.out.buffer, stdout); + puts ("info: *** subprocess output ends ***"); + + TEST_VERIFY + (strcmp (proc.out.buffer, + "tst-test_compare.c:28: numeric comparison failure\n" + " left: 1 (0x1); from: ch\n" + " right: -1 (0xffffffff); from: -1\n" + "tst-test_compare.c:29: numeric comparison failure\n" + " left: 2 (0x2); from: 2LL\n" + " right: -2 (0xfffffffffffffffe); from: -2LL\n" + "tst-test_compare.c:30: numeric comparison failure\n" + " left: 3 (0x3); from: 3L\n" + " right: -3 (0xfffd); from: (short) -3\n") == 0); + + /* Check that there is no output on standard error. */ + support_capture_subprocess_check (&proc, "TEST_COMPARE", 0, sc_allow_stdout); + + return 0; +} + +#include