[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#655518: gcc-4.6 on Alpha: incorrect code generation when compiling git



Package: gcc-4.6
Version: 4.6.2-11
Severity: important
User: debian-alpha@lists.debian.org
Usertags: alpha
X-Debbugs-CC: debian-alpha@lists.debian.org

git_1:1.7.8.3-1 FTBFS because of incorrect code generation by gcc-4.6.
The test suite of git fails with a segmentation violation as revealed by
running it under gdb:


Program terminated with signal 11, Segmentation fault.
#0  0x00000001200a44b4 in git_config_rename_section (
    old_name=0x11fa8392d "branch.vier", new_name=0x11fa83939 "branch.zwei")
    at config.c:1533
1533						output[0] = '\t';
(gdb) list
1528						 * a declaration to put on the
1529						 * next line; indent with a
1530						 * tab
1531						 */
1532						output -= 1;
1533						output[0] = '\t';
1534					}
1535				}
1536				remove = 0;
1537			}
(gdb) print output
$1 = 0x11fa83000 "z = 1\n"
(gdb) print output[0]
$2 = 122 'z'
(gdb) print offset
$3 = 16
(gdb) print i
$4 = <optimized out>

A test higher up in the code ensures at this point that output points
into a char array so it is safe to decrement it and deference as the
code above does.  Also the print statements above reveal that output
points to valid characters within a bigger char array as expected,
nevertheless a segmentation violation is encountered when writing to
string[0].

The generated object code is:

1525					if (strlen(output) > 0) {
   0x00000001200a448c <+956>:	lda	t1,1(a1)
   0x00000001200a4490 <+960>:	ldq_u	t2,0(a1)
   0x00000001200a4494 <+964>:	extqh	t2,t1,t1
   0x00000001200a4498 <+968>:	sra	t1,0x38,t1
   0x00000001200a449c <+972>:	beq	t1,0x1200a4228
<git_config_rename_section+344>

1526						/*
1527						 * More content means there's
1528						 * a declaration to put on the
1529						 * next line; indent with a
---Type <return> to continue, or q <return> to quit---
1530						 * tab
1531						 */
1532						output -= 1;
   0x00000001200a44bc <+1004>:	lda	a1,-1(a1)

1533						output[0] = '\t';
   0x00000001200a44a0 <+976>:	lda	t1,1
   0x00000001200a44a4 <+980>:	sll	t1,0x3d,t1
   0x00000001200a44a8 <+984>:	addq	a1,t1,t0
   0x00000001200a44ac <+988>:	lda	t3,-1(t0)
   0x00000001200a44b0 <+992>:	lda	t1,9
=> 0x00000001200a44b4 <+996>:	ldq_u	t2,-1(t0)
   0x00000001200a44b8 <+1000>:	insbl	t1,t3,t1
   0x00000001200a44c0 <+1008>:	mskbl	t2,t3,t2
   0x00000001200a44c4 <+1012>:	or	t1,t2,t1
   0x00000001200a44c8 <+1016>:	stq_u	t1,-1(t0)
   0x00000001200a44cc <+1020>:	br	0x1200a4228
<git_config_rename_section+344>

At line 1532 it decrements register a1 and saves back to a1, but does
not use that result when saving '\t" to output[0]!  Instead it sets the
high bit of a1 (the lda t1,1; sll t1,0x3d,t1; addq a1,t1,t0
instructions) and then uses that as the memory address for storing the
'\t'.  But that is no longer in user memory space!

If I compile with the -mcpu=ev67 compiler option (mainly to allow
compilation with the byte-word extension) then the generated code is:

                                       output -= 1;
                                        output[0] = '\t';
 22c:   09 00 3f 20     lda     t0,9
                                         * More content means there's
                                         * a declaration to put on the
                                         * next line; indent with a
                                         * tab
                                         */
                                        output -= 1;
 230:   ff ff 31 22     lda     a1,-1(a1)
                                        output[0] = '\t';
 234:   ff ff 22 38     stb     t0,-1(t1)
 238:   c6 ff ff c3     br      154 <git_config_rename_section+0x154>


It is now clear that the compiler has lost the connection between the
decrement of output and the following store to output.  The decrement to
output leaves the result in register a1, but the store to output uses
register t1!

The code compiles correctly with gcc-4.4 and gcc-4.5 so this is gcc-4.6
specific.

I attach the source code with just the function that gets incorrectly
compiled.  It still requires an extra header file that is part of the
git source, so can't be compiled directly, sorry.  My attempt to reduce
it further into a minimal self contained unit no longer exhibited the
incorrect compilation.

Cheers
Michael.
/*
 * GIT - The information manager from hell
 *
 * Copyright (C) Linus Torvalds, 2005
 * Copyright (C) Johannes Schindelin, 2005
 *
 */

#include "cache.h"

extern int section_name_match(const char *, const char *);
extern int store_write_section(const int, const char *);
extern int write_error(const char *);
extern int store;

/* if new_name == NULL, the section is removed instead */
int git_config_rename_section(const char *old_name, const char *new_name)
{
	int ret = 0, remove = 0;
	char *config_filename;
	struct lock_file *lock = xcalloc(sizeof(struct lock_file), 1);
	int out_fd;
	char buf[1024];
	FILE *config_file;

	if (config_exclusive_filename)
		config_filename = xstrdup(config_exclusive_filename);
	else
		config_filename = git_pathdup("config");
	out_fd = hold_lock_file_for_update(lock, config_filename, 0);
	if (out_fd < 0) {
		ret = error("could not lock config file %s", config_filename);
		goto out;
	}

	if (!(config_file = fopen(config_filename, "rb"))) {
		/* no config file means nothing to rename, no error */
		goto unlock_and_out;
	}

	while (fgets(buf, sizeof(buf), config_file)) {
		int i;
		int length;
		char *output = buf;
		for (i = 0; buf[i] && isspace(buf[i]); i++)
			; /* do nothing */
		if (buf[i] == '[') {
			/* it's a section */
			int offset = section_name_match(&buf[i], old_name);
			if (offset > 0) {
				ret++;
				if (new_name == NULL) {
					remove = 1;
					continue;
				}
				store = strlen(new_name);
				if (!store_write_section(out_fd, new_name)) {
					ret = write_error(lock->filename);
					goto out;
				}
				/*
				 * We wrote out the new section, with
				 * a newline, now skip the old
				 * section's length
				 */
				output += offset + i;
				if (strlen(output) > 0) {
					/*
					 * More content means there's
					 * a declaration to put on the
					 * next line; indent with a
					 * tab
					 */
					output -= 1;
					output[0] = '\t';
				}
			}
			remove = 0;
		}
		if (remove)
			continue;
		length = strlen(output);
		if (write_in_full(out_fd, output, length) != length) {
			ret = write_error(lock->filename);
			goto out;
		}
	}
	fclose(config_file);
unlock_and_out:
	if (commit_lock_file(lock) < 0)
		ret = error("could not commit config file %s", config_filename);
out:
	free(config_filename);
	return ret;
}

Reply to: