Discussion:
[libav-devel] [GASPP PATCH 1/2] Fix the regexp used for replacing .align
Martin Storsjo
2018-10-19 21:07:51 UTC
Permalink
The condition above allows multiple spaces, but the actual replacement
only allowed one space.
---
gas-preprocessor.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gas-preprocessor.pl b/gas-preprocessor.pl
index c56301c..b22ee8a 100755
--- a/gas-preprocessor.pl
+++ b/gas-preprocessor.pl
@@ -922,7 +922,7 @@ sub handle_serialized_line {
# ALIGN in armasm syntax is the actual number of bytes
if ($line =~ /\.(?:p2)?align\s+(\d+)/) {
my $align = 1 << $1;
- $line =~ s/\.(?:p2)?align\s(\d+)/ALIGN $align/;
+ $line =~ s/\.(?:p2)?align\s+(\d+)/ALIGN $align/;
}
# Convert gas style [r0, :128] into armasm [***@128] alignment specification
$line =~ s/\[([^\[,]+),?\s*:(\d+)\]/[$1\@$2]/g;
--
2.7.4
Martin Storsjo
2018-10-19 21:07:52 UTC
Permalink
For cases like "b 1b", this is matched as $cond = " ". This fixes
preprocessing with a preprocessor that preserves multiple spaces as such,
which cl.exe does.
---
gas-preprocessor.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gas-preprocessor.pl b/gas-preprocessor.pl
index b22ee8a..e7a135d 100755
--- a/gas-preprocessor.pl
+++ b/gas-preprocessor.pl
@@ -885,7 +885,7 @@ sub handle_serialized_line {
my $width = $4;
my $target = $5;
# Don't interpret e.g. bic as b<cc> with ic as conditional code
- if ($cond !~ /^(|$arm_cond_codes)$/) {
+ if ($cond !~ /^(\s*|$arm_cond_codes)$/) {
# Not actually a branch
} elsif ($target =~ /^(\d+)([bf])$/) {
# The target is a local label
--
2.7.4
Martin Storsjo
2018-10-19 21:18:27 UTC
Permalink
For cases like "b 1b", this could previously be matched as
$cond = " ".

This fixes preprocessing with a preprocessor that preserves multiple
consecutive spaces, like cl.exe does.
---
Better fix, which also works in a number of cases where the previous
version failed.
---
gas-preprocessor.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gas-preprocessor.pl b/gas-preprocessor.pl
index b22ee8a..c42412f 100755
--- a/gas-preprocessor.pl
+++ b/gas-preprocessor.pl
@@ -879,7 +879,7 @@ sub handle_serialized_line {


# Check branch instructions
- if ($line =~ /(?:^|\n)\s*(\w+\s*:\s*)?(bl?x?\.?(..)?(\.w)?)\s+(\w+)/) {
+ if ($line =~ /(?:^|\n)\s*(\w+\s*:\s*)?(bl?x?\.?([^\s]{2})?(\.w)?)\s+(\w+)/) {
my $instr = $2;
my $cond = $3;
my $width = $4;
--
2.7.4
Luca Barbato
2018-10-22 15:39:12 UTC
Permalink
Post by Martin Storsjo
For cases like "b 1b", this could previously be matched as
$cond = " ".
This fixes preprocessing with a preprocessor that preserves multiple
consecutive spaces, like cl.exe does.
---
Better fix, which also works in a number of cases where the previous
version failed.
---
gas-preprocessor.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gas-preprocessor.pl b/gas-preprocessor.pl
index b22ee8a..c42412f 100755
--- a/gas-preprocessor.pl
+++ b/gas-preprocessor.pl
@@ -879,7 +879,7 @@ sub handle_serialized_line {
# Check branch instructions
- if ($line =~ /(?:^|\n)\s*(\w+\s*:\s*)?(bl?x?\.?(..)?(\.w)?)\s+(\w+)/) {
+ if ($line =~ /(?:^|\n)\s*(\w+\s*:\s*)?(bl?x?\.?([^\s]{2})?(\.w)?)\s+(\w+)/) {
my $instr = $2;
my $cond = $3;
my $width = $4;
Looks fine.
Janne Grunau
2018-10-22 18:03:33 UTC
Permalink
Post by Martin Storsjo
For cases like "b 1b", this could previously be matched as
$cond = " ".
This fixes preprocessing with a preprocessor that preserves multiple
consecutive spaces, like cl.exe does.
---
Better fix, which also works in a number of cases where the previous
version failed.
---
gas-preprocessor.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gas-preprocessor.pl b/gas-preprocessor.pl
index b22ee8a..c42412f 100755
--- a/gas-preprocessor.pl
+++ b/gas-preprocessor.pl
@@ -879,7 +879,7 @@ sub handle_serialized_line {
# Check branch instructions
- if ($line =~ /(?:^|\n)\s*(\w+\s*:\s*)?(bl?x?\.?(..)?(\.w)?)\s+(\w+)/) {
+ if ($line =~ /(?:^|\n)\s*(\w+\s*:\s*)?(bl?x?\.?([^\s]{2})?(\.w)?)\s+(\w+)/) {
my $instr = $2;
my $cond = $3;
my $width = $4;
both ok

Janne

Luca Barbato
2018-10-22 15:46:07 UTC
Permalink
Post by Martin Storsjo
The condition above allows multiple spaces, but the actual replacement
only allowed one space.
---
gas-preprocessor.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gas-preprocessor.pl b/gas-preprocessor.pl
index c56301c..b22ee8a 100755
--- a/gas-preprocessor.pl
+++ b/gas-preprocessor.pl
@@ -922,7 +922,7 @@ sub handle_serialized_line {
# ALIGN in armasm syntax is the actual number of bytes
if ($line =~ /\.(?:p2)?align\s+(\d+)/) {
my $align = 1 << $1;
- $line =~ s/\.(?:p2)?align\s(\d+)/ALIGN $align/;
+ $line =~ s/\.(?:p2)?align\s+(\d+)/ALIGN $align/;
}
I guess it is fine.
Loading...