From patchwork Wed Nov 9 21:36:47 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Fran=C3=A7ois_Dumont?= X-Patchwork-Id: 81573 Delivered-To: patch@linaro.org Received: by 10.140.97.165 with SMTP id m34csp417158qge; Wed, 9 Nov 2016 13:37:35 -0800 (PST) X-Received: by 10.99.67.7 with SMTP id q7mr18281551pga.74.1478727455593; Wed, 09 Nov 2016 13:37:35 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id p1si1101868pav.178.2016.11.09.13.37.35 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 09 Nov 2016 13:37:35 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-440900-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-440900-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-440900-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=gmail.com DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=Z31vkob8I6+EMw7pI JEz0LlEdDdD9Zg8lGaibR7hbgIdOYVQT5wpwQGoEOMbme5pErL4IGVb/E8bXhJdd iw/LTJWryyhY9TXW0mXKhEgIxjTqsuU/cIodHmUpsnb1Nf/drH7xbreffFO6ne9r C61j1qmIqe4VTNvqwOCJlmlI/8= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=1/qBtjPiNynvJn/mwVt2bVV kIbM=; b=QHx73RAtqsbgHEmKQBFWw2WId67ffuvxiXZsHj5iRoYFE4H475h4tSr weXCqBVPfT8L7464B/OmbjKsdSRsWiq66BH9ojymo5Q+9KL+e83nTadf2/As/Z9J UuD7T+vg6ocrUJi3yWtO2sJturqkzl3e4IWTjMRIVJ5waMucOwyQ= Received: (qmail 106654 invoked by alias); 9 Nov 2016 21:37:02 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 106625 invoked by uid 89); 9 Nov 2016 21:37:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=confirms, debugcc, fmt, debug.cc X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-wm0-f66.google.com Received: from mail-wm0-f66.google.com (HELO mail-wm0-f66.google.com) (74.125.82.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 09 Nov 2016 21:36:51 +0000 Received: by mail-wm0-f66.google.com with SMTP id u144so31821558wmu.0; Wed, 09 Nov 2016 13:36:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to; bh=gcVyFqkvE1Zwhv8OZ6Hz4fOPwKETLmsCCi318NVh1HE=; b=FjOIEffcKsYxhSrGSjPZ/dvJtJZiHycOFaibPEE9/H303r1lD4G0lrhzbea7cNJ3df 7VCurW3IfAzkTj8tFPOTnESHkDpJQg9ioSFtlur5xlFhvZ8UgZBOyS5lIWiMMVH9jEE5 tgqeVxDFOdbm5mdnRugexCYpqurrEIDvJyZkleSsJ7a5TPMMfTSA9JY1uVKz9eBucfU2 I4zklurKFBPIvCdYuXNzrue5c0upHsdCa27W4ncywZ7skjqjCTJJz4R86zdWv7eFXYMC JGdOOr9CswWNHQzLfk8IFKgXym3Os9qG33DzpkheDYGXQSlnwCta+6sZohPCnxP29ga6 JwdA== X-Gm-Message-State: ABUngvfoiOTWuWR9nShaa9EIiG9FLt7pW5ZcR1FtPmlm9ftcxr+08KYF7ILdKGDS8nTO+A== X-Received: by 10.195.13.70 with SMTP id ew6mr1468678wjd.89.1478727409324; Wed, 09 Nov 2016 13:36:49 -0800 (PST) Received: from [192.168.0.37] (arf62-1-82-237-250-248.fbx.proxad.net. [82.237.250.248]) by smtp.googlemail.com with ESMTPSA id d64sm9256177wmh.3.2016.11.09.13.36.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 09 Nov 2016 13:36:48 -0800 (PST) Subject: Review debug message generation To: "libstdc++@gcc.gnu.org" , gcc-patches References: From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= Message-ID: Date: Wed, 9 Nov 2016 22:36:47 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Hi Here is a proposal to review how we generate the debug output in case of assertion failure. It removes usage of format_word which, as a side effect will fix PR 77459. Should I reference this PR in the ChangeLog ? I introduced a print_literal function to avoid using strlen on them. I know that gcc optimizes calls to strlen on literals but in our case we were not directly calling it on the literals. Tested under Linux x86_64, ok to commit ? * src/c++11/debug.cc (format_word): Delete. (print_literal): New. Replace call to print_word for literals. François On 08/11/2016 22:35, fdumont at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77459 > > --- Comment #8 from François Dumont --- > Ok, at least it confirms what I thought about builtins. So the problem is > rather a buggy target. > > Even if so I'll try to find an alternative approach to avoid snprintf usage. > diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc index d79e43b..2b9c00b 100644 --- a/libstdc++-v3/src/c++11/debug.cc +++ b/libstdc++-v3/src/c++11/debug.cc @@ -546,11 +546,6 @@ namespace using _Error_formatter = __gnu_debug::_Error_formatter; using _Parameter = __gnu_debug::_Error_formatter::_Parameter; - template - int - format_word(char* buf, int n, const char* fmt, _Tp s) - { return std::min(__builtin_snprintf(buf, n, fmt, s), n - 1); } - void get_max_length(std::size_t& max_length) { @@ -577,6 +572,11 @@ namespace bool _M_wordwrap; }; + template + void + print_literal(PrintContext& ctx, const char(&word)[Length]) + { print_word(ctx, word, Length - 1); } + void print_word(PrintContext& ctx, const char* word, std::ptrdiff_t count = -1) @@ -627,18 +627,19 @@ namespace } else { - print_word(ctx, "\n", 1); + print_literal(ctx, "\n"); print_word(ctx, word, count); } } + template void print_type(PrintContext& ctx, const type_info* info, - const char* unknown_name) + const char(&unknown_name)[Length]) { if (!info) - print_word(ctx, unknown_name); + print_literal(ctx, unknown_name); else { int status; @@ -784,20 +785,18 @@ namespace { if (type._M_name) { - const int bufsize = 64; - char buf[bufsize]; - int written - = format_word(buf, bufsize, "\"%s\"", type._M_name); - print_word(ctx, buf, written); + print_literal(ctx, "\""); + print_word(ctx, type._M_name); + print_literal(ctx, "\""); } - print_word(ctx, " {\n"); + print_literal(ctx, " {\n"); if (type._M_type) { - print_word(ctx, " type = "); + print_literal(ctx, " type = "); print_type(ctx, type._M_type, ""); - print_word(ctx, ";\n"); + print_literal(ctx, ";\n"); } } @@ -809,9 +808,9 @@ namespace if (inst._M_name) { - int written - = format_word(buf, bufsize, "\"%s\" ", inst._M_name); - print_word(ctx, buf, written); + print_literal(ctx, "\""); + print_word(ctx, inst._M_name); + print_literal(ctx, "\" "); } int written @@ -820,7 +819,7 @@ namespace if (inst._M_type) { - print_word(ctx, " type = "); + print_literal(ctx, " type = "); print_type(ctx, inst._M_type, ""); } } @@ -838,36 +837,36 @@ namespace { const auto& ite = variant._M_iterator; - print_word(ctx, "iterator "); + print_literal(ctx, "iterator "); print_description(ctx, ite); if (ite._M_type) { if (ite._M_constness != _Error_formatter::__unknown_constness) { - print_word(ctx, " ("); + print_literal(ctx, " ("); print_field(ctx, param, "constness"); - print_word(ctx, " iterator)"); + print_literal(ctx, " iterator)"); } - print_word(ctx, ";\n"); + print_literal(ctx, ";\n"); } if (ite._M_state != _Error_formatter::__unknown_state) { - print_word(ctx, " state = "); + print_literal(ctx, " state = "); print_field(ctx, param, "state"); - print_word(ctx, ";\n"); + print_literal(ctx, ";\n"); } if (ite._M_sequence) { - print_word(ctx, " references sequence "); + print_literal(ctx, " references sequence "); if (ite._M_seq_type) { - print_word(ctx, "with type '"); + print_literal(ctx, "with type '"); print_field(ctx, param, "seq_type"); - print_word(ctx, "' "); + print_literal(ctx, "' "); } int written @@ -875,34 +874,34 @@ namespace print_word(ctx, buf, written); } - print_word(ctx, "}\n", 2); + print_literal(ctx, "}\n"); } break; case _Parameter::__sequence: - print_word(ctx, "sequence "); + print_literal(ctx, "sequence "); print_description(ctx, variant._M_sequence); if (variant._M_sequence._M_type) - print_word(ctx, ";\n", 2); + print_literal(ctx, ";\n"); - print_word(ctx, "}\n", 2); + print_literal(ctx, "}\n"); break; case _Parameter::__instance: - print_word(ctx, "instance "); + print_literal(ctx, "instance "); print_description(ctx, variant._M_instance); if (variant._M_instance._M_type) - print_word(ctx, ";\n", 2); + print_literal(ctx, ";\n"); - print_word(ctx, "}\n", 2); + print_literal(ctx, "}\n"); break; case _Parameter::__iterator_value_type: - print_word(ctx, "iterator::value_type "); + print_literal(ctx, "iterator::value_type "); print_description(ctx, variant._M_iterator_value_type); - print_word(ctx, "}\n", 2); + print_literal(ctx, "}\n"); break; default: @@ -1020,38 +1019,36 @@ namespace __gnu_debug void _Error_formatter::_M_error() const { - const int bufsize = 128; - char buf[bufsize]; - // Emit file & line number information bool go_to_next_line = false; PrintContext ctx; if (_M_file) { - int written = format_word(buf, bufsize, "%s:", _M_file); - print_word(ctx, buf, written); + print_word(ctx, _M_file); + print_literal(ctx, ":"); go_to_next_line = true; } if (_M_line > 0) { + char buf[64]; int written = __builtin_sprintf(buf, "%u:", _M_line); print_word(ctx, buf, written); go_to_next_line = true; } if (go_to_next_line) - print_word(ctx, "\n", 1); + print_literal(ctx, "\n"); if (ctx._M_max_length) ctx._M_wordwrap = true; - print_word(ctx, "Error: "); + print_literal(ctx, "Error: "); // Print the error message assert(_M_text); print_string(ctx, _M_text, _M_parameters, _M_num_parameters); - print_word(ctx, ".\n", 2); + print_literal(ctx, ".\n"); // Emit descriptions of the objects involved in the operation ctx._M_first_line = true; @@ -1067,7 +1064,7 @@ namespace __gnu_debug case _Parameter::__iterator_value_type: if (!has_header) { - print_word(ctx, "\nObjects involved in the operation:\n"); + print_literal(ctx, "\nObjects involved in the operation:\n"); has_header = true; } print_description(ctx, _M_parameters[i]);