From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-19.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3845FC3405E for ; Wed, 19 Feb 2020 17:44:46 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DB9EF206DB for ; Wed, 19 Feb 2020 17:44:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="ezxabv3h" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DB9EF206DB Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 41A156B0003; Wed, 19 Feb 2020 12:44:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3CB556B0006; Wed, 19 Feb 2020 12:44:45 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2E10B6B0007; Wed, 19 Feb 2020 12:44:45 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0007.hostedemail.com [216.40.44.7]) by kanga.kvack.org (Postfix) with ESMTP id 15AE16B0003 for ; Wed, 19 Feb 2020 12:44:45 -0500 (EST) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id C27C02463 for ; Wed, 19 Feb 2020 17:44:44 +0000 (UTC) X-FDA: 76507601688.25.tooth58_295a358388222 X-HE-Tag: tooth58_295a358388222 X-Filterd-Recvd-Size: 6753 Received: from mail-pl1-f193.google.com (mail-pl1-f193.google.com [209.85.214.193]) by imf12.hostedemail.com (Postfix) with ESMTP for ; Wed, 19 Feb 2020 17:44:44 +0000 (UTC) Received: by mail-pl1-f193.google.com with SMTP id y8so345879pll.13 for ; Wed, 19 Feb 2020 09:44:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=1Zerwr5LVoD9w0G00G1YsmnKd+u52a5G2yVAzT73CuQ=; b=ezxabv3hlXaPKMrbyel92GCxKOdqEfJfkncTl9L1UwG6liI1neuaZftEVFBsHwlFGi ildP/XITFnjN+SBQsZOKh01ai+2P7DSunNk4ZxKbBkdxzVx5E2JFakDUiOwg2LzOmv6g kOn0EEwkPhT5wmsYUqUuG03F9EajzmECYOKF/X3EmUva/uldBTWbjlnlxTB/bufIEoEA f39vAdwOJe1DBsTeNBwAth2+QIXKU4Z383+etxjE9atpUHqRsLcFta4Z8kYt6MrFzQcw aOVgxiFcJxB0D/PMlQ1uKNxNRTolrBFoXqNOdRp5kMpKrH7MMqL9KAUM1eWk1NDA5CFy eKRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=1Zerwr5LVoD9w0G00G1YsmnKd+u52a5G2yVAzT73CuQ=; b=pc4EdfZ5cbMvWXrQfmAEhTA5usqrWu7UP7rZDV7riZS1aLr8/yqs+JJtf60PJFB9CT 2VJSF3wyf24yLXwC9mm4mmwPn5eZjvwBvxNSlACZ7APX9vLdupjvgbbqkbmKarIOuJ5c MC4iOqB1PHBOSs937kfJZijsGjvWug44ufNmoLal1+Lx8MmZeWqwjRnPxT8YwpZWr0Uw jCu/1uglNv+r6L918tn8qjxc0YShbwIcZONSP9T7xM15Ce8s81oK4Nh/uW4aJkn2fx3w PojrK6ZblB/8ZpkipPrBzGralXR5j7Hw5OeWa0AXVLoOpWlrVfoHZH13xRRTuWCpTbzU v5wA== X-Gm-Message-State: APjAAAWHCfLv3vNm76OrzQLElfWncYb4biQSXlUYCUPjlnJQaNSf0Bd9 a0TYh7C1SKT7f1bzsjnZ9rpq/xo7AqnV/3eb0LpdyA== X-Google-Smtp-Source: APXvYqwgwaJwqCbIGiJG/t9AN5GA3UaekcGZBPD9itMUe9n7Ox0RNcbqUdR+MZqmN6x0VvswiWfk9cfEyYh8GYYpsCY= X-Received: by 2002:a17:902:8a88:: with SMTP id p8mr26529137plo.179.1582134282820; Wed, 19 Feb 2020 09:44:42 -0800 (PST) MIME-Version: 1.0 References: <20200219045423.54190-1-natechancellor@gmail.com> <20200219045423.54190-4-natechancellor@gmail.com> <20200219093445.386f1c09@gandalf.local.home> In-Reply-To: <20200219093445.386f1c09@gandalf.local.home> From: Nick Desaulniers Date: Wed, 19 Feb 2020 09:44:31 -0800 Message-ID: Subject: Re: [PATCH 3/6] tracing: Wrap section comparison in tracer_alloc_buffers with COMPARE_SECTIONS To: Steven Rostedt , Nathan Chancellor Cc: Masahiro Yamada , Michal Marek , Arnd Bergmann , Ingo Molnar , Jason Baron , Catalin Marinas , Andrew Morton , LKML , Linux Kbuild mailing list , linux-arch , Linux Memory Management List , clang-built-linux Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Feb 19, 2020 at 6:34 AM Steven Rostedt wrote: > > On Tue, 18 Feb 2020 21:54:20 -0700 > Nathan Chancellor wrote: > > > Clang warns: > > > > ../kernel/trace/trace.c:9335:33: warning: array comparison always > > evaluates to true [-Wtautological-compare] > > if (__stop___trace_bprintk_fmt != __start___trace_bprintk_fmt) > > ^ > > 1 warning generated. > > > > These are not true arrays, they are linker defined symbols, which are > > just addresses so there is not a real issue here. Use the > > COMPARE_SECTIONS macro to silence this warning by casting the linker > > defined symbols to unsigned long, which keeps the logic the same. > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/765 > > Signed-off-by: Nathan Chancellor > > --- > > kernel/trace/trace.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > > index c797a15a1fc7..e1f3b16e457b 100644 > > --- a/kernel/trace/trace.c > > +++ b/kernel/trace/trace.c > > @@ -9332,7 +9332,7 @@ __init static int tracer_alloc_buffers(void) > > goto out_free_buffer_mask; > > > > /* Only allocate trace_printk buffers if a trace_printk exists */ > > - if (__stop___trace_bprintk_fmt != __start___trace_bprintk_fmt) > > + if (COMPARE_SECTIONS(__stop___trace_bprintk_fmt, !=, __start___trace_bprintk_fmt)) > > Sorry, but this is really ugly and unreadable. Please find some other > solution to fix this. > > NAK-by: Steven Rostedt > Hey Nathan, Thanks for the series; enabling the warning will help us find more bugs. Revisiting what the warning is about, I checked on this "referring to symbols defined in linker scripts from C" pattern. This document [0] (by no means definitive, but I think it has a good idea) says: Linker symbols that represent a data address: In C code, declare the variable as an extern variable. Then, refer to the value of the linker symbol using the & operator. Because the variable is at a valid data address, we know that a data pointer can represent the value. Linker symbols for an arbitrary address: In C code, declare this as an extern symbol. The type does not matter. If you are using GCC extensions, declare it as "extern void". Indeed, it seems that Clang is happier with that pattern: https://godbolt.org/z/sW3t5W Looking at __stop___trace_bprintk_fmt in particular: kernel/trace/trace.h 1923:extern const char *__stop___trace_bprintk_fmt[]; (Should be `extern const void __stop___trace_bprintk_fmt;` void since we don't access any data or function from that symbol, just compare its address) kernel/trace/trace_printk.c 260: start_index = __stop___trace_bprintk_fmt - __start___trace_bprintk_fmt; (Should be `start_index = (uintptr_t)__stop___trace_bprintk_fmt - (uintptr_t)__start___trace_bprintk_fmt;`) (storing the result in an int worries me a little, but maybe that refactoring can be saved for another day) kernel/trace/trace.c 9335: if (__stop___trace_bprintk_fmt != __start___trace_bprintk_fmt) (Should be `if (&__stop___trace_bprintk_fmt - &__start___trace_bprintk_fmt)`. That's not a significant change to the existing code, and is quite legible IMO) Steven, thoughts? [0] http://downloads.ti.com/docs/esd/SPRUI03/using-linker-symbols-in-c-c-applications-slau1318080.html -- Thanks, ~Nick Desaulniers