mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [PATCH 1/5] Pull a couple common AWK functions for CFI scripts into separate file
Date: Thu, 8 Oct 2015 17:37:21 -0400	[thread overview]
Message-ID: <20151008213721.GA8645@brightrain.aerifal.cx> (raw)
In-Reply-To: <1443785556-27473-1-git-send-email-alexinbeijing@gmail.com>

On Fri, Oct 02, 2015 at 01:32:32PM +0200, Alex Dowad wrote:
> There is a lot which could be common between i386 and x86_64, but none of it
> will be useful for any other arch. These should be useful for all archs,
> however.
> ---

Thanks! I'm commenting on these as a set rather than breaking this
down into separate replies.

I think you still have errors in your instruction parsing which will
lead to incorrect clobbers, just not as bad as before. For example,
movl $0,(%esp,%esi) gets marked as clobbering %esi. Using .* in the
patterns is probably a bad idea. I think the x86_64 patch is also
affected, but I tested it and otherwise it basically works.

In any case I'm going ahead and applying patches 1-3, with 2 and 3
merged into one commit. In general, try to merge changes that are
conceptually related and where putting them together in one patch
doesn't obscure what was changed; for example, merging 4 with 2-3
would obscure changes IMO since there would be pattern changes and
operand order changes in the same lines. I've also edited commit
messages to match capitalization used in shortlog and to make git log
fit in 80 columns (generally messages should be wrapped at 70-72
chars, like email).

Could you follow up with fixes for patches 4 and 5? I'd like to try to
get these into the next release (soon).

Rich


      parent reply	other threads:[~2015-10-08 21:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-02 11:32 Alex Dowad
2015-10-02 11:32 ` [PATCH 2/5] When generating CFI for i386 asm, don't mistake an FDIV instruction for DIV Alex Dowad
2015-10-02 11:32 ` [PATCH 3/5] When generating CFI for i386 asm, don't mistake an FMUL instruction for MUL Alex Dowad
2015-10-02 11:32 ` [PATCH 4/5] In i386 CFI script, binary ops like ADD, AND, etc. modify the 2nd operand, not 1st Alex Dowad
2015-10-02 11:32 ` [PATCH 5/5] Add script to add CFI directives to asm files in debug builds of x86_64 Alex Dowad
2015-10-08 21:37 ` Rich Felker [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151008213721.GA8645@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).