From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c09::241; helo=mail-wm0-x241.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-x241.google.com (mail-wm0-x241.google.com [IPv6:2a00:1450:400c:c09::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A4E54210D9797 for ; Tue, 19 Jun 2018 06:06:52 -0700 (PDT) Received: by mail-wm0-x241.google.com with SMTP id e16-v6so294013wmd.0 for ; Tue, 19 Jun 2018 06:06:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=SSwBlUii3K5ZbUEzc/wGgfWncOJKYGZ6Sbzg3tao/fo=; b=c0Gze6+O3n5B8nFuqQPXeL8O0mIi+RRcm09V8L72s8lxEKkzojamTrscDTROBAG35T 4jMuIJ2VjP4A5BLQHthrFVAwLq23HyU1IIy1jfLd6fG7qN0pup3q2ysViKJMb5w777oI CRDK6IFnY3UP+CpOdYWJwaIP83TAvORxPn3mg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=SSwBlUii3K5ZbUEzc/wGgfWncOJKYGZ6Sbzg3tao/fo=; b=N3mPFomIolwzhyy2FHV4bXe2zz9ci+Jnf886z7HvSko4dU+2BRpdkvrNBnPk+aK2ex b26/+KyHpOmllYdCp/RqxURq6lsU6D/XCcFQ7pTVy1zk0YkcV7VZNDqomfMw36CHpbCj SAizMNPEWia+oIBJkLgwcV+exjWGzcD3569IFSnZ1qiPgg+kUg3zdLkLlMRAV2eM/8Z/ eJ2fcBiM57e6XMVxVkFov+OlCMAS+kfssyNWrktbVeZv5vA5GflYVaX/JkMXFFvLp4+i pC4eZQGNxnw7PT4IySXLuRVhHN1q3F7AbdvLCavv8949fJGd2InZNhLpaC8eCssEv5kX quFQ== X-Gm-Message-State: APt69E3/gf8oFBAjYo/fJkiIx/PGEV9xeCn4wFVRyy3Ir5F2auZfxVtJ o4LzY0WcN3y/e6Hjdy+F7qXJB6Nof4k= X-Google-Smtp-Source: ADUXVKIQu4ESO4qbcCsgjMDrkZxJ58KDQ4MQDjX93CGPUaJdH9L1ISI2l4fQbCF9s6fPHk+IMGarbA== X-Received: by 2002:a1c:91c4:: with SMTP id t187-v6mr12647170wmd.51.1529413611153; Tue, 19 Jun 2018 06:06:51 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id q70-v6sm15140wmd.45.2018.06.19.06.06.49 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 19 Jun 2018 06:06:49 -0700 (PDT) Date: Tue, 19 Jun 2018 14:06:48 +0100 From: Leif Lindholm To: Laszlo Ersek Cc: Ard Biesheuvel , edk2-devel@lists.01.org Message-ID: <20180619130648.z3vbxxwewf5ciq5o@bivouac.eciton.net> References: <20180618204918.31835-1-ard.biesheuvel@linaro.org> <20180618215758.vsoetpww4ax64v6g@bivouac.eciton.net> <96210413-f241-63d1-3e96-a77757d3b0e4@redhat.com> <20180619095411.52siakmtbfokc6mu@bivouac.eciton.net> <6e536d26-a1bb-52f2-ceca-8b39e35fe5ce@redhat.com> MIME-Version: 1.0 In-Reply-To: <6e536d26-a1bb-52f2-ceca-8b39e35fe5ce@redhat.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH] EmbeddedPkg/GdbSerialLib: avoid left shift of negative quantity X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jun 2018 13:06:53 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Jun 19, 2018 at 02:51:41PM +0200, Laszlo Ersek wrote: > On 06/19/18 11:54, Leif Lindholm wrote: > > On Tue, Jun 19, 2018 at 12:51:49AM +0200, Laszlo Ersek wrote: > >> On 06/18/18 23:57, Leif Lindholm wrote: > >>> On Mon, Jun 18, 2018 at 10:49:18PM +0200, Ard Biesheuvel wrote: > >>>> Clang complains about left shifting a negative value being undefined. > >>> > >>> As well it should. > >>> > >>>> EmbeddedPkg/Library/GdbSerialLib/GdbSerialLib.c:151:30: > >>>> error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value] > >>>> OutputData = (UINT8)((~DLAB<<7)|((BreakSet<<6)|((Parity<<3)|((StopBits<<2)| Data)))); > >>>> > >>>> Redefine all bit pattern constants as unsigned to work around this. > >>> > >>> So, I'm totally OK with this and > >>> Reviewed-by: Leif Lindholm > >>> but ... > >>> would it be worth fixing up BIT0-31 in Base.h and use those? > >> > >> If we started with the BITxx macros now, I'd agree. Given the current > >> tree, I don't :) I've made an argument against the suggestion earlier: > >> > >> UINT64 Value64; > >> > >> Value64 = ~0x1; > >> Value64 = ~0x1U; > >> > >> The two assignments produce different results. > >> > >> In the first case, the integer constant 0x1 has type "int". Our C > >> language implementation uses, for type "int", two's complement > >> representation, 1 sign bit, 31 value bits, and no padding bits. So after > >> the bitwise complement, we get 0x1111_1111_1111_1110, also with type > >> int, and with value (-2). Taking UINT64 for "unsigned long int" or > >> "unsigned long long int", after the conversion implied by the assignment > >> we get ((UINT64_MAX + 1) + (-2)), aka (UINT64_MAX - 1). > > > > Err, if by UINT64_MAX + 1 you mean 0, > > No, I don't. > > > as a way of introducing the > > second part, I'm with you. Otherwise I think I need to go back to > > school. > > I really meant (UINT64_MAX + 1) as the mathematical integer > 18,446,744,073,709,551,616; without the modular wrap-around that occurs > in C. The modular wrap-around of C is defined in terms of the > mathematical values. This is how the ISO C99 standard defines the behavior: > > ------ > 6.3.1.3 Signed and unsigned integers > > 1 When a value with integer type is converted to another integer type > other than _Bool, if the value can be represented by the new type, it > is unchanged. > > 2 Otherwise, if the new type is unsigned, the value is converted by > repeatedly adding or subtracting one more than the maximum value that > can be represented in the new type until the value is in the range of > the new type. [footnote 49] So for negative add, for positive subtract? I find the language in the spec is indicative of the people who write it. And those people spend a lot of time working on compiler optimisations. > 3 Otherwise, the new type is signed and the value cannot be represented > in it; either the result is implementation-defined or an > implementation-defined signal is raised. *twitch* > [footnote 49] The rules describe arithmetic on the mathematical value, > not the value of a given type of expression. > ------ > > Therefore we really have (18,446,744,073,709,551,616 + (-2)) in the > above reasoning. Blimey. Live and learn. Thanks for that - at least the period in school was shorter than I feared. > >> In the second case, the constant 0x1U has type "unsigned int". After the > >> bitwise complement, we get (UINT32_MAX - 1), also of type "unsigned > >> int". Taking UINT64 for "unsigned long int" or "unsigned long long int", > >> after the conversion implied by assignment we get the exact same value, > >> (UINT32_MAX - 1). > > > > But these examples make it look like the differences are an > > unobservable internal side effect. That's not the case. > > Hm, sorry, I don't understand. My point was that the difference was > extremely observable, because in the first assignment, we end up with > value 18,446,744,073,709,551,614; in the second, we end up with > 4,294,967,294. > > Perhaps my expression "exact same value" was misleading. Yes, that threw me. Combined with your detestation for sign-extension (which does help describe the situation without going full spec). > There I > referred to 6.3.1.3p1, which I've now quoted above as well -- I meant > that converting 4,294,967,294 from "unsigned int" to either of "unsigned > long int" or "unsigned long long int" didn't change the value. Understood. Thanks a lot! / Leif