From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by sourceware.org (Postfix) with ESMTPS id 90F493858D38 for ; Fri, 28 Oct 2022 18:23:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 90F493858D38 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f49.google.com with SMTP id m29-20020a05600c3b1d00b003c6bf423c71so7116165wms.0 for ; Fri, 28 Oct 2022 11:23:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=9pWvFooEJxYzXe6GPr9HKaqi1i1M4gN64pmeRjP0hho=; b=bMW0xI4NYw4wTtx1pcnfwQznxyfaomm9mKhv0JECj5TgPz4+GTkEmyTKaQ76eH7cfl dtTWGXDcWeNJaeViQzF7yRXsb2VKhyaTBaFg4J2QgkAW3NUJRSAATzNkWY1Cx4nLjDbi YeAyfZa8sMxx20s0RZQSADjPxxKuS3egnbDQgQlI4S4Pofy8LCBkHu2a+BZNFgu1n59u m8sVUfYFSU6zFF2bE3WwxUTMyY5sOGmiboesYSEXs04coW47aTkuBAmdSEJqXz+Gxktj Ad/0yEyBuBtx3HBBogxmKFXOs5R1IQvlSW3u2I0ExqHyMShScH/+26Omx4vYsFJUC1qN HhSw== X-Gm-Message-State: ACrzQf0OgAWQTPNK8gNwUZ+csRM/MmQFPnuuLGMOOVfoVJxlLwLFU5Ou cAEhtT9C8mAHby//43q3b8bVX9cGxjRaOQ== X-Google-Smtp-Source: AMsMyM4c2437UtAz0tLliwEQDJjJOKlz/CdBSe02iMG0vNBO8eBWURdF16KnJTDFg6l/sxK0WxXuRg== X-Received: by 2002:a05:600c:19c7:b0:3cd:1d9c:518d with SMTP id u7-20020a05600c19c700b003cd1d9c518dmr10655605wmq.85.1666981434168; Fri, 28 Oct 2022 11:23:54 -0700 (PDT) Received: from ?IPv6:2001:8a0:f93a:3b00:e038:5cdc:b8bf:4653? ([2001:8a0:f93a:3b00:e038:5cdc:b8bf:4653]) by smtp.gmail.com with ESMTPSA id e4-20020a5d5004000000b0023655e51c33sm4201447wrt.4.2022.10.28.11.23.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 28 Oct 2022 11:23:53 -0700 (PDT) Subject: Re: [PATCH v3] enum_flags to_string (was: Re: [PATCH v2 07/29] Thread options & clone events (core + remote)) To: Simon Marchi , gdb-patches@sourceware.org References: <20220713222433.374898-1-pedro@palves.net> <20220713222433.374898-8-pedro@palves.net> <5d68cd36-e8d6-e8ad-5428-863e79742062@simark.ca> <6b91f8f6-d3b5-c44b-297c-ce1c3a1c80f6@palves.net> <6aee3c00-b334-97e2-62f7-0434822bcf7a@simark.ca> From: Pedro Alves Message-ID: <47ef6c92-c4f6-448f-c4c3-71da4618107c@palves.net> Date: Fri, 28 Oct 2022 19:23:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <6aee3c00-b334-97e2-62f7-0434822bcf7a@simark.ca> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,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 2022-10-28 4:59 p.m., Simon Marchi wrote: > On 2022-10-28 07 h 08, Pedro Alves wrote: >> On 2022-10-28 11:26 a.m., Pedro Alves wrote: ... >> + /* Check string conversion. */ >> + { >> + SELF_CHECK (to_string (0) == "0x0 []"); >> + SELF_CHECK (to_string (FLAG1) == "0x2 [FLAG1]"); >> + SELF_CHECK (to_string (FLAG1 | FLAG3) == "0xa [FLAG1 FLAG3]"); >> + SELF_CHECK (to_string (FLAG1 | FLAG2 | FLAG3) == "0xe [FLAG1 FLAG3 0x4]"); >> + SELF_CHECK (to_string (FLAG2) == "0x4 [0x4]"); >> + SELF_CHECK (to_string (test_flag (0xff)) == "0xff [FLAG1 FLAG3 0xf5]"); > > Ish, this crashes with: > > /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/enum-flags.h:191:12: runtime error: load of value 255, which is not a valid value for type 'test_flag' Ah. That's because I missed that test_flag doesn't have an explicit underlying type, as in "enum test_flag : underlying". The rules are different if there's an explicit underlying type or not. enums with an explicit type can hold any value of the underlying type. Best to fix the testcase. We could either remove that particular test, which is not really that important, as what it tests is exercised by the previous two tests, or switch to test_uflags instead, which does have an explicit underlying type. I did the latter. > > with --enable-ubsan. We can disable UBSan per function using > __attribute__((no_sanitize_undefined)) or > __attribute__((no_sanitize("undefined"))). With clang, it appears > possible to push and pop function attributes, but it doesn't seem to be > possible with gcc. > > I had a hard time identifying which functions needed it, so I slapped it > on all functions in enum-flags.h, and the test passed fine, so that's > good news. Here's my change, to get you started: I'd rather fix the test. Here's another version of the patch. Nth time a charm. :-) -- >8 -- >From cc536203a1898e04dfa96cb56df10cbf7f1fa63d Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 25 Oct 2022 15:39:37 +0100 Subject: [PATCH v4] enum_flags to_string This commit introduces shared infrastructure that can be used to implement enum_flags -> to_string functions. With this, if we want to support converting a given enum_flags specialization to string, we just need to implement a function that provides the enumerator->string mapping, like so: enum some_flag { SOME_FLAG1 = 1 << 0, SOME_FLAG2 = 1 << 1, SOME_FLAG3 = 1 << 2, }; DEF_ENUM_FLAGS_TYPE (some_flag, some_flags); static std::string to_string (some_flags flags) { static constexpr some_flags::string_mapping mapping[] = { MAP_ENUM_FLAG (SOME_FLAG1), MAP_ENUM_FLAG (SOME_FLAG2), MAP_ENUM_FLAG (SOME_FLAG3), }; return flags.to_string (mapping); } .. and then to_string(SOME_FLAG2 | SOME_FLAG3) produces a string like "0x6 [SOME_FLAG2 SOME_FLAG3]". If we happen to forget to update the mapping array when we introduce a new enumerator, then the string representation will pretty-print the flags it knows about, and then the leftover flags in hex (one single number). For example, if we had missed mapping SOME_FLAG2 above, we'd end up with: to_string(SOME_FLAG2 | SOME_FLAG3) => "0x6 [SOME_FLAG2 0x4]"); Other than in the unit tests included, no actual usage of the functionality is added in this commit. Change-Id: I835de43c33d13bc0c95132f42c3f97318b875779 --- gdb/unittests/enum-flags-selftests.c | 24 ++++++++++++ gdbsupport/enum-flags.h | 57 ++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/gdb/unittests/enum-flags-selftests.c b/gdb/unittests/enum-flags-selftests.c index f52fc7220a1..b427a99a8f5 100644 --- a/gdb/unittests/enum-flags-selftests.c +++ b/gdb/unittests/enum-flags-selftests.c @@ -374,6 +374,20 @@ enum test_uflag : unsigned DEF_ENUM_FLAGS_TYPE (test_flag, test_flags); DEF_ENUM_FLAGS_TYPE (test_uflag, test_uflags); +/* to_string enumerator->string mapping function used to test + enum_flags::to_string. This intentionally misses mapping one + enumerator (FLAG2). */ + +static std::string +to_string (test_uflags flags) +{ + static constexpr test_uflags::string_mapping mapping[] = { + MAP_ENUM_FLAG (UFLAG1), + MAP_ENUM_FLAG (UFLAG3), + }; + return flags.to_string (mapping); +} + static void self_test () { @@ -581,6 +595,16 @@ self_test () SELF_CHECK (ok); } + + /* Check string conversion. */ + { + SELF_CHECK (to_string (0) == "0x0 []"); + SELF_CHECK (to_string (UFLAG1) == "0x2 [UFLAG1]"); + SELF_CHECK (to_string (UFLAG1 | UFLAG3) == "0xa [UFLAG1 UFLAG3]"); + SELF_CHECK (to_string (UFLAG1 | UFLAG2 | UFLAG3) == "0xe [UFLAG1 UFLAG3 0x4]"); + SELF_CHECK (to_string (UFLAG2) == "0x4 [0x4]"); + SELF_CHECK (to_string (test_uflag (0xff)) == "0xff [UFLAG1 UFLAG3 0xf5]"); + } } } /* namespace enum_flags_tests */ diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h index cd500f55ff3..911f3fd7890 100644 --- a/gdbsupport/enum-flags.h +++ b/gdbsupport/enum-flags.h @@ -130,6 +130,17 @@ class enum_flags typedef E enum_type; typedef typename enum_underlying_type::type underlying_type; + /* For to_string. Maps one enumerator of E to a string. */ + struct string_mapping + { + E flag; + const char *str; + }; + + /* Convenience for to_string implementations, to build a + string_mapping array. */ +#define MAP_ENUM_FLAG(ENUM_FLAG) { ENUM_FLAG, #ENUM_FLAG } + public: /* Allow default construction. */ constexpr enum_flags () @@ -183,6 +194,52 @@ class enum_flags /* Binary operations involving some unrelated type (which would be a bug) are implemented as non-members, and deleted. */ + /* Convert this object to a std::string, using MAPPING as + enumerator-to-string mapping array. This is not meant to be + called directly. Instead, enum_flags specializations should have + their own to_string function wrapping this one, thus hidding the + mapping array from callers. */ + + template + std::string + to_string (const string_mapping (&mapping)[N]) const + { + enum_type flags = raw (); + std::string res = hex_string (flags); + res += " ["; + + bool need_space = false; + for (const auto &entry : mapping) + { + if ((flags & entry.flag) != 0) + { + /* Do op~ in the underlying type, because if enum_type's + underlying type is signed, op~ won't be defined for + it. */ + flags &= (enum_type) ~(underlying_type) entry.flag; + + if (need_space) + res += " "; + res += entry.str; + + need_space = true; + } + } + + /* If there were flags not included in the mapping, print them as + a hex number. */ + if (flags != 0) + { + if (need_space) + res += " "; + res += hex_string (flags); + } + + res += "]"; + + return res; + } + private: /* Stored as enum_type because GDB knows to print the bit flags neatly if the enum values look like bit flags. */ base-commit: 508ccf9b3e1db355037a4a1c9004efe0d6d3ffbf -- 2.36.0