Re: [Tinycc-devel] Small patch

2013-01-31 Thread Thomas Preud'homme 
Le jeudi 31 janvier 2013 02:07:36, Domingo Alvarez Duarte a écrit :
 Also here :
 
 static void asm_expr_logic(TCCState *s1, ExprValue *pe)
 {
 int op;
 ExprValue e2;
 
 asm_expr_prod(s1, pe);
 for(;;) {
 op = tok;
 if (op != ''  op != '|'  op != '^')
 break;
 next();
 asm_expr_prod(s1, e2);
 if (pe-sym || e2.sym)
 tcc_error(invalid operation with label);
 switch(op) {
 case '':
 pe-v = e2.v;
 break;
 case '|':
 pe-v |= e2.v;
 break;
 default:
 case '^':  ///what this case
 after default mean 
 pe-v ^= e2.v;
 break;
 }
 }
 }

Looks weird indeed but I am reluctant to change it when I don't know why it 
was done this way in the first place. Since the file was commited in one go, I 
can't see if this result from a mistake or if it was intentional. We can 
consider changing this and the other such example right after the release.

Thomas


signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Thomas Preud'homme 
Le jeudi 31 janvier 2013 00:08:19, Ivo van Poorten a écrit :
 Error:
 
 $ ./configure --cc=tcc
 Binary  directory   /usr/local/bin
 TinyCC directory/usr/local/lib/tcc
 Library directory   /usr/local/lib
 Include directory   /usr/local/include
 Manual directory/usr/local/share/man
 Info directory  /usr/local/share/info
 Doc directory   /usr/local/share/doc/tcc
 Target root prefix
 Source path  /home/ivo/git/tinycc
 C compiler   tcc
 Target OSLinux
 CPU  x86
 Big Endian   no
 gprof enabledno
 cross compilers  no
 use libgcc   no
 Creating config.mak and config.h
 $ make
 [..snip..]
 gcc -c lib/bcheck.c -o bcheck.o -I. -I/home/ivo/git/tinycc -Wall -g -O2
 -mpreferred-stack-boundary=2 -m386 -malign-functions=0 -m32 cc1: error:
 unrecognized command line option -m386
 
 
 88888888
 
 diff --git a/Makefile b/Makefile
 index d257464..0333ebe 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -232,7 +232,7 @@ libtcc1.a : FORCE
  lib/%/libtcc1.a : FORCE $(PROGS_CROSS)
 @$(MAKE) -C lib cross TARGET=$*
  bcheck.o : lib/bcheck.c
 -   gcc -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
 +   $(CC) -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
  FORCE:
 
  # install

Unfortunetely it still breaks because of a wrong detection of GCC_MAJOR. 
Because the test to determine the major version number of gcc fails, GCC_MAJOR 
is set to 2. I made the following patch but I'm still not sure how to handle 
flags when the compiler is not gcc.

Shall no flag be set and the project fail because of missing -I for instance?

I don't have a clear idea for now.

Best regards,

Thomas
diff --git a/Makefile b/Makefile
index d6a0a28..ce37592 100644
--- a/Makefile
+++ b/Makefile
@@ -13,6 +13,7 @@ CFLAGS_P=$(CFLAGS) -pg -static
 LIBS_P=
 LDFLAGS_P=$(LDFLAGS)
 
+ifdef GCC_MAJOR
 ifneq ($(GCC_MAJOR),2)
 CFLAGS+=-fno-strict-aliasing
 ifneq ($(GCC_MAJOR),3)
@@ -30,6 +31,7 @@ CFLAGS+=-march=i386 -falign-functions=0
 endif
 endif
 endif
+endif
 
 ifdef CONFIG_WIN64
 CONFIG_WIN32=yes
@@ -234,7 +236,7 @@ libtcc1.a : FORCE
 lib/%/libtcc1.a : FORCE $(PROGS_CROSS)
 	@$(MAKE) -C lib cross TARGET=$*
 bcheck.o : lib/bcheck.c
-	gcc -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
+	$(CC) -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
 FORCE:
 
 # install
diff --git a/configure b/configure
index fa2c97d..289b8ca 100755
--- a/configure
+++ b/configure
@@ -231,6 +231,21 @@ esac
 
 fi
 
+# check compiler is gcc
+cat  $TMPC EOF
+int main(void) {
+#ifdef __GNUC__
+return 0;
+#else
+#error compiler is not gcc
+#endif
+}
+EOF
+
+if $cc -o $TMPO $TMPC 2 /dev/null ; then
+gcc_major=2
+fi
+
 # check gcc version
 cat  $TMPC EOF
 int main(void) {
@@ -242,7 +257,6 @@ return 0;
 }
 EOF
 
-gcc_major=2
 if $cc -o $TMPO $TMPC 2 /dev/null ; then
 gcc_major=3
 fi
@@ -409,11 +423,13 @@ print_var2 CONFIG_TCC_LIBPATHS $tcc_libpaths
 print_var2 CONFIG_TCC_CRTPREFIX $tcc_crtprefix
 print_var2 CONFIG_TCC_ELFINTERP $tcc_elfinterp
 
-echo #define GCC_MAJOR $gcc_major  $TMPH
+if [ x$gcc_major != x ] ; then
+echo #define GCC_MAJOR $gcc_major  $TMPH
+echo GCC_MAJOR=$gcc_major  config.mak
+fi
 
 cat  config.mak EOF
 CC=$cc
-GCC_MAJOR=$gcc_major
 HOST_CC=$host_cc
 AR=$ar
 STRIP=$strip -s -R .comment -R .note


signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Thomas Preud'homme 
Le jeudi 31 janvier 2013 02:19:38, Domingo Alvarez Duarte a écrit :
 Here is some that I found simple to make:
 
 Author: mingodad mingo...@gmail.com  2013-01-31 01:17:50
 Committer: mingodad mingo...@gmail.com  2013-01-31 01:17:50
 Parent: 1b1e7ee1fd2f269872128dc5e8b830bd55dfa80c (Fix cross-compilation
 out-of-tree build)
 Branch: vio_patch
 Follows: release_0_9_25
 Precedes:
 
 Alerts found using Coverity Scan
 
 --- libtcc.c
 ---
 index b0a9b1a..69d9e0d 100644
 @@ -479,8 +479,7 @@ ST_FUNC void put_extern_sym2(Sym *sym, Section
 *section, case TOK_strlen:
  case TOK_strcpy:
  case TOK_alloca:
 -strcpy(buf, __bound_);
 -strcat(buf, name);
 +snprintf(buf, sizeof(buf), __bound_%s, name);
  name = buf;
  break;
  }

strcpy and strcat are C89 and C99 while snprintf is only C99.

 
 --- tccelf.c
 ---
 index a4dee19..479b2fa 100644
 @@ -114,7 +114,7 @@ ST_FUNC int put_elf_sym(Section *s, uplong value,
 unsigned long size,
  if (ELFW(ST_BIND)(info) != STB_LOCAL) {
  /* add another hashing entry */
  nbuckets = base[0];
 -h = elf_hash(name) % nbuckets;
 +h = name ? elf_hash(name) % nbuckets : 0;
  *ptr = base[2 + h];
  base[2 + h] = sym_index;
  base[1]++;

Fixed (see just pushed commits)

 @@ -3052,7 +3052,7 @@ static int ld_add_file_list(TCCState *s1, const char
 *cmd, int as_needed)
  ret = -1;
  goto lib_parse_error;
  }
 -strcpy(libname, filename[1]);
 +snprintf(libname, sizeof(libname), %s, filename[1]);
  libname_to_filename(s1, libname, filename);
  } else if (t != LD_TOK_NAME) {
  tcc_error_noabort(filename expected);

C99

 
 --- tccgen.c
 ---
 index 4300403..e06a932 100644
 @@ -361,6 +361,7 @@ void vpush64(int ty, unsigned long long v)
  CValue cval;
  CType ctype;
  ctype.t = ty;
 +ctype.ref = 0;
  cval.ull = v;
  vsetc(ctype, VT_CONST, cval);
  }
 @@ -1734,6 +1735,7 @@ ST_FUNC void gen_op(int op)
  }
  vswap();
  type1.t = t;
 +type1.ref = 0;
  gen_cast(type1);
  vswap();
  /* special case for shifts and long long: we keep the shift as
 @@ -2717,6 +2719,7 @@ static void struct_decl(CType *type, int u)
  v = anon_sym++;
  }
  type1.t = a;
 +type1.ref = 0;
  /* we put an undefined size for struct/union */
  s = sym_push(v | SYM_STRUCT, type1, 0, -1);
  s-r = 0; /* default alignment is zero as gcc */
 @@ -3396,6 +3399,7 @@ static void gfunc_param_typed(Sym *func, Sym *arg)
  /* default casting : only need to convert float to double */
  if ((vtop-type.t  VT_BTYPE) == VT_FLOAT) {
  type.t = VT_DOUBLE;
 +type.ref = 0;
  gen_cast(type);
  }
  } else if (arg == NULL) {
 @@ -3592,6 +3596,7 @@ ST_FUNC void unary(void)
  if ((vtop-r  (VT_VALMASK | VT_LVAL | VT_SYM)) == VT_CONST) {
  CType boolean;
  boolean.t = VT_BOOL;
 +boolean.ref = 0;
  gen_cast(boolean);
  vtop-c.i = !vtop-c.i;
  } else if ((vtop-r  VT_VALMASK) == VT_CMP)
 @@ -4101,6 +4106,7 @@ static void expr_cond(void)
  CType boolean;
  int c;
  boolean.t = VT_BOOL;
 +boolean.ref = 0;
  vdup();
  gen_cast(boolean);
  c = vtop-c.i;

All fixed.

 @@ -5574,7 +5580,7 @@ ST_FUNC void gen_inline_functions(void)
  str = fn-token_str;
  fn-sym = NULL;
  if (file)
 -strcpy(file-filename, fn-filename);
 +snprintf(file-filename, sizeof(file-filename), %s,
 fn-filename);
  sym-r = VT_SYM | VT_CONST;
  sym-type.t = ~VT_INLINE;

C99


signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Stephan Beal 
On Thu, Jan 31, 2013 at 12:07 PM, Thomas Preud'homme robo...@celest.frwrote:

  -strcpy(buf, __bound_);
  -strcat(buf, name);
  +snprintf(buf, sizeof(buf), __bound_%s, name);


strcpy and strcat are C89 and C99 while snprintf is only C99.


The semantics of the above variants are not the same, are they? strcpy()
and strcat() are both writing to the same address in buf, i.e. strcat is
overwriting what strcpy() copied into buf. So the end result, unless i'm
sorely mistaken, is a copy of the name with the __bound_ prefix. strncat()
is c89, BTW.

-- 
- stephan beal
http://wanderinghorse.net/home/stephan/
http://gplus.to/sgbeal
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Thomas Preud'homme 
Le jeudi 31 janvier 2013 12:34:27, Stephan Beal a écrit :
 On Thu, Jan 31, 2013 at 12:07 PM, Thomas Preud'homme 
robo...@celest.frwrote:
   -strcpy(buf, __bound_);
   -strcat(buf, name);
   +snprintf(buf, sizeof(buf), __bound_%s, name);
 
 strcpy and strcat are C89 and C99 while snprintf is only C99.
 
 
 The semantics of the above variants are not the same, are they? strcpy()
 and strcat() are both writing to the same address in buf, i.e. strcat is
 overwriting what strcpy() copied into buf. So the end result, unless i'm
 sorely mistaken, is a copy of the name with the __bound_ prefix. strncat()
 is c89, BTW.

strcat copy the second argument at the end of the string pointed at by the 
first argument.

So after strcpy you'll have __bound_ and then the strcat will add name at the 
end of this string.

Did I misunderstand what you said?

Best regards,

Thomas


signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread grischka 

Thomas Preud'homme wrote:

 bcheck.o : lib/bcheck.c
-   gcc -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
+   $(CC) -c $ -o $@ $(CPPFLAGS) $(CFLAGS)


Unfortunetely it still breaks because of a wrong detection of GCC_MAJOR. 
Because the test to determine the major version number of gcc fails, GCC_MAJOR 
is set to 2. I made the following patch but I'm still not sure how to handle 
flags when the compiler is not gcc.


Shall no flag be set and the project fail because of missing -I for instance?

I don't have a clear idea for now.


Maybe we should just put bcheck.o into libtcc1.a (and thus make via
the rules of lib/Makefile).  Unless there is a reason why we
shouldn't.


-h = elf_hash(name) % nbuckets;
+h = name ? elf_hash(name) % nbuckets : 0;


This is counterproductive IMO because when you read this you get
the wrong impression that there are cases where name is NULL.
But in fact there is no such case.  So the naive reader is misled,
while the knowledgeable reader must conclude that the writer didn't
know what s/he was doing.


 boolean.t = VT_BOOL;
+boolean.ref = 0;


Also redundant because type.ref is used with type.t = VT_FUNC only.

I'm not completely opposed to make this look more solid but then
such patch should address the problem systematically such that the
code becomes smaller, not bigger.


-parse_btype(btype, ad);
+if (parse_btype(btype, ad))
+expect(type);


Breaks tcc.  Never push without test, in particular not during the
calm down period before release. ;)


 -strcpy(buf, __bound_);
 -strcat(buf, name);
 +snprintf(buf, sizeof(buf), __bound_%s, name);


There are cases where we might want to use pstrcpy instead of strcpy.
This is no such case because __bound_memcpy cannot overflow buf[32].
Same with
pstrcpy(buf, sizeof buf, a.out);
Because a.out cannot overflow buf[1024].

--- grischka


___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Thomas Preud'homme 
Le jeudi 31 janvier 2013 13:19:59, grischka a écrit :
 Thomas Preud'homme wrote:
   bcheck.o : lib/bcheck.c
  
  -   gcc -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
  +   $(CC) -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
  
  Unfortunetely it still breaks because of a wrong detection of GCC_MAJOR.
  Because the test to determine the major version number of gcc fails,
  GCC_MAJOR is set to 2. I made the following patch but I'm still not sure
  how to handle flags when the compiler is not gcc.
  
  Shall no flag be set and the project fail because of missing -I for
  instance?
  
  I don't have a clear idea for now.
 
 Maybe we should just put bcheck.o into libtcc1.a (and thus make via
 the rules of lib/Makefile).  Unless there is a reason why we
 shouldn't.

Yes but that doesn't change the general assumption that we are compiling tcc 
with gcc. If --cc is specified at configure time, then CFLAGS and CPPFLAGS 
should be set when running make (make CPPFLAGS=foo CFLAGS=bar)

 
  -h = elf_hash(name) % nbuckets;
  +h = name ? elf_hash(name) % nbuckets : 0;
 
 This is counterproductive IMO because when you read this you get
 the wrong impression that there are cases where name is NULL.
 But in fact there is no such case.  So the naive reader is misled,
 while the knowledgeable reader must conclude that the writer didn't
 know what s/he was doing.

Well, the test at the top of the function suggested me name could be NULL. I 
suppose it can't be NULL when the symbol is global or common. What about a 
malformed elf file?

 
   boolean.t = VT_BOOL;
  
  +boolean.ref = 0;
 
 Also redundant because type.ref is used with type.t = VT_FUNC only.
 
 I'm not completely opposed to make this look more solid but then
 such patch should address the problem systematically such that the
 code becomes smaller, not bigger.

Can you develop about what you have in mind?

 
  -parse_btype(btype, ad);
  +if (parse_btype(btype, ad))
  +expect(type);
 
 Breaks tcc.  Never push without test, in particular not during the
 calm down period before release. ;)

Yes, you are absolutely right. I tried on a simple example (where tcc fails to 
issue a warning) and I was obviously wrong. You were right from the beginning, 
I shall stop doing any commits unless a very serious bug is reported and keep 
things for after the release.

 
   -strcpy(buf, __bound_);
   -strcat(buf, name);
   +snprintf(buf, sizeof(buf), __bound_%s, name);
 
 There are cases where we might want to use pstrcpy instead of strcpy.
 This is no such case because __bound_memcpy cannot overflow buf[32].
 Same with
   pstrcpy(buf, sizeof buf, a.out);
 Because a.out cannot overflow buf[1024].

sprintf could be used  then to save some space

 
 --- grischka

Thomas


signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Domingo Alvarez Duarte 
On Thu, Jan 31, 2013 at 12:19 PM, grischka gris...@gmx.de wrote:



  -h = elf_hash(name) % nbuckets;
 +h = name ? elf_hash(name) % nbuckets : 0;


 This is counterproductive IMO because when you read this you get
 the wrong impression that there are cases where name is NULL.
 But in fact there is no such case.  So the naive reader is misled,
 while the knowledgeable reader must conclude that the writer didn't
 know what s/he was doing.


I totally agree with you !
But look some lines above on this same function !

/* return the symbol number */
ST_FUNC int put_elf_sym(Section *s, uplong value, unsigned long size,
int info, int other, int shndx, const char *name)
{
int name_offset, sym_index;
int nbuckets, h;
ElfW(Sym) *sym;
Section *hs;

sym = section_ptr_add(s, sizeof(ElfW(Sym)));
if (name)
/ This mean
name can be null ?
name_offset = put_elf_str(s-link, name);
else
name_offset = 0;
/* XXX: endianness */
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Domingo Alvarez Duarte 
On Thu, Jan 31, 2013 at 12:36 PM, Thomas Preud'homme robo...@celest.frwrote:

 Le jeudi 31 janvier 2013 13:19:59, grischka a écrit :
 
-strcpy(buf, __bound_);
-strcat(buf, name);
+snprintf(buf, sizeof(buf), __bound_%s, name);
 
  There are cases where we might want to use pstrcpy instead of strcpy.
  This is no such case because __bound_memcpy cannot overflow buf[32].
  Same with
pstrcpy(buf, sizeof buf, a.out);
  Because a.out cannot overflow buf[1024].

 sprintf could be used  then to save some space

 
  --- grischka


Maybe you didn't noticed that snprintf guarantee no buffer overflow while
sprintf doesn't 
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread grischka 

Vincent Lefevre wrote:

On 2013-01-31 10:52:14 +0100, Thomas Preud'homme wrote:

Le jeudi 31 janvier 2013 02:07:36, Domingo Alvarez Duarte a écrit :

switch(op) {
case '':
pe-v = e2.v;
break;
case '|':
pe-v |= e2.v;
break;
default:
case '^':  ///what this case
after default mean 
pe-v ^= e2.v;
break;
}
Looks weird indeed but I am reluctant to change it when I don't know why it 
was done this way in the first place. Since the file was commited in one go, I 
can't see if this result from a mistake or if it was intentional. We can 
consider changing this and the other such example right after the release.


Perhaps there was a default: by default the author forgot to remove.
And as it is probably just useless (if op can only be '', '|' or '^'),
no-one noticed it.


I guess it was to avoid a compiler warning such as
case X not handled in switch

--- grischka


___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Vincent Lefevre 
On 2013-01-31 13:22:22 +0100, grischka wrote:
 Vincent Lefevre wrote:
 Perhaps there was a default: by default the author forgot to remove.
 And as it is probably just useless (if op can only be '', '|' or '^'),
 no-one noticed it.
 
 I guess it was to avoid a compiler warning such as
   case X not handled in switch

In such a case, a default: at the end with an assertion failure
would have been better.

-- 
Vincent Lefèvre vinc...@vinc17.net - Web: http://www.vinc17.net/
100% accessible validated (X)HTML - Blog: http://www.vinc17.net/blog/
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Thomas Preud'homme 
Le jeudi 31 janvier 2013 13:45:28, Domingo Alvarez Duarte a écrit :
 
 Maybe you didn't noticed that snprintf guarantee no buffer overflow while
 sprintf doesn't 

I didn't. Grischka just explained how no overflow can occured so sprintf is 
fine.

Thomas


signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread grischka 

Vincent Lefevre wrote:

On 2013-01-31 13:22:22 +0100, grischka wrote:

Vincent Lefevre wrote:

Perhaps there was a default: by default the author forgot to remove.
And as it is probably just useless (if op can only be '', '|' or '^'),
no-one noticed it.

I guess it was to avoid a compiler warning such as
case X not handled in switch


In such a case, a default: at the end with an assertion failure
would have been better.



No.  An assertion only makes sense to make sure about what we aren't
entirely sure.  In this case we are sure (that default never happens).

--- grischka


___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread grischka 

Thomas Preud'homme wrote:

Maybe we should just put bcheck.o into libtcc1.a (and thus make via
the rules of lib/Makefile).  Unless there is a reason why we
shouldn't.


Yes but that doesn't change the general assumption that we are compiling tcc 
with gcc. If --cc is specified at configure time, then CFLAGS and CPPFLAGS 
should be set when running make (make CPPFLAGS=foo CFLAGS=bar)


Sure, but if you want to solve the --cc=tcc/clang problem then the
question with using what compiler for libtcc1.a needs to be answered,
and also whether it makes sense to treat bcheck.o separately.

Well, the test at the top of the function suggested me name could be NULL. I 
suppose it can't be NULL when the symbol is global or common. What about a 
malformed elf file?


Are you sure that putting an hash link to an not-existing name entry
is the right way to deal with a malformed elf file?


 boolean.t = VT_BOOL;

+boolean.ref = 0;

Also redundant because type.ref is used with type.t = VT_FUNC only.

I'm not completely opposed to make this look more solid but then
such patch should address the problem systematically such that the
code becomes smaller, not bigger.


Can you develop about what you have in mind?


No, I just suspect.  There is mostly always a better way than
to copy  paste equivalent statements into several places, the
more if the statements are altogether logically redundant.


Same with
pstrcpy(buf, sizeof buf, a.out);
Because a.out cannot overflow buf[1024].


sprintf could be used  then to save some space


I pushed some changes to that.

--- grischka


___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Vincent Lefevre 
On 2013-01-31 14:50:38 +0100, grischka wrote:
 Vincent Lefevre wrote:
 On 2013-01-31 13:22:22 +0100, grischka wrote:
 Vincent Lefevre wrote:
 Perhaps there was a default: by default the author forgot to remove.
 And as it is probably just useless (if op can only be '', '|' or '^'),
 no-one noticed it.
 I guess it was to avoid a compiler warning such as
 case X not handled in switch
 
 In such a case, a default: at the end with an assertion failure
 would have been better.
 
 No.  An assertion only makes sense to make sure about what we aren't
 entirely sure.  In this case we are sure (that default never happens).

You are sure *now*. But imagine that in the future you modify the if
at the beginning of the loop to add a 4th operator but you forget to
modify the switch...

BTW, since you are sure now and think an assert is useless anyway,
removing the default: (or adding an empty default: at the end)
now should be completely safe.

-- 
Vincent Lefèvre vinc...@vinc17.net - Web: http://www.vinc17.net/
100% accessible validated (X)HTML - Blog: http://www.vinc17.net/blog/
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Thomas Preud'homme 
Le jeudi 31 janvier 2013 14:53:42, grischka a écrit :
 Thomas Preud'homme wrote:
  Maybe we should just put bcheck.o into libtcc1.a (and thus make via
  the rules of lib/Makefile).  Unless there is a reason why we
  shouldn't.
  
  Yes but that doesn't change the general assumption that we are compiling
  tcc with gcc. If --cc is specified at configure time, then CFLAGS and
  CPPFLAGS should be set when running make (make CPPFLAGS=foo CFLAGS=bar)
 
 Sure, but if you want to solve the --cc=tcc/clang problem then the
 question with using what compiler for libtcc1.a needs to be answered,
 and also whether it makes sense to treat bcheck.o separately.

Sure.

 
 Are you sure that putting an hash link to an not-existing name entry
 is the right way to deal with a malformed elf file?

No, again you're right. I got over zealous in trying to address anything that 
was shown to me. I should have taken the cautious approach and later look at 
all the surrounding code. Thanks for your vigilance ;)

Best regards,

Thomas


signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Michael Matz 
Hi,

On Thu, 31 Jan 2013, Vincent Lefevre wrote:

  In such a case, a default: at the end with an assertion failure
  would have been better.
  
  No.  An assertion only makes sense to make sure about what we aren't
  entirely sure.  In this case we are sure (that default never happens).
 
 You are sure *now*. But imagine that in the future you modify the if
 at the beginning of the loop to add a 4th operator but you forget to
 modify the switch...
 
 BTW, since you are sure now and think an assert is useless anyway,
 removing the default: (or adding an empty default: at the end)
 now should be completely safe.

It will lead to slower and/or larger code to add a default case with a
separate body.  The current way of doing it is IMO the exactly correct
way.  If people are really confused by that, put a comment next to the
'default:'.

Generally stylistic warnings of coverity (and actually not just
stylistic things) have to be taken with a huge amount of salt.


Ciao,
Michael.

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread Vincent Lefevre 
On 2013-01-31 15:56:34 +0100, Michael Matz wrote:
 It will lead to slower and/or larger code to add a default case with a
 separate body.  The current way of doing it is IMO the exactly correct
 way.  If people are really confused by that, put a comment next to the
 'default:'.

OK (until compilers become smart enough to track possible values
of variables). I would rather see the case '^' as a comment
rather than code. Or add a special assert that would be checked
only in some debug mode.

-- 
Vincent Lefèvre vinc...@vinc17.net - Web: http://www.vinc17.net/
100% accessible validated (X)HTML - Blog: http://www.vinc17.net/blog/
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-31 Thread grischka 

Vincent Lefevre wrote:

No.  An assertion only makes sense to make sure about what we aren't
entirely sure.  In this case we are sure (that default never happens).


You are sure *now*. But imagine that in the future you modify the if
at the beginning of the loop to add a 4th operator but you forget to
modify the switch...


If I'd add that 4th operator then I'm sure it would actually work.


BTW, since you are sure now and think an assert is useless anyway,
removing the default: (or adding an empty default: at the end)
now should be completely safe.


Geez.  Everybody knows that TinyCC isn't the recommended way to
write a compiler.

--- grischka


___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

[Tinycc-devel] Small patch

2013-01-30 Thread Ivo van Poorten 
Error:

$ ./configure --cc=tcc
Binary  directory   /usr/local/bin
TinyCC directory/usr/local/lib/tcc
Library directory   /usr/local/lib
Include directory   /usr/local/include
Manual directory/usr/local/share/man
Info directory  /usr/local/share/info
Doc directory   /usr/local/share/doc/tcc
Target root prefix  
Source path  /home/ivo/git/tinycc
C compiler   tcc
Target OSLinux
CPU  x86
Big Endian   no
gprof enabledno
cross compilers  no
use libgcc   no
Creating config.mak and config.h
$ make
[..snip..]
gcc -c lib/bcheck.c -o bcheck.o -I. -I/home/ivo/git/tinycc -Wall -g -O2
-mpreferred-stack-boundary=2 -m386 -malign-functions=0 -m32 cc1: error:
unrecognized command line option -m386


88888888

diff --git a/Makefile b/Makefile
index d257464..0333ebe 100644
--- a/Makefile
+++ b/Makefile
@@ -232,7 +232,7 @@ libtcc1.a : FORCE
 lib/%/libtcc1.a : FORCE $(PROGS_CROSS)
@$(MAKE) -C lib cross TARGET=$*
 bcheck.o : lib/bcheck.c
-   gcc -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
+   $(CC) -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
 FORCE:
 
 # install

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-30 Thread Domingo Alvarez Duarte 
Hello !
I'm looking again on the reports done with Coverity Scan and found several
things one of it is this:

static void asm_expr_prod(TCCState *s1, ExprValue *pe)
{
int op;
ExprValue e2;

asm_expr_unary(s1, pe);
for(;;) {
op = tok;
if (op != '*'  op != '/'  op != '%' 
op != TOK_SHL  op != TOK_SAR)
break;
next();
asm_expr_unary(s1, e2);
if (pe-sym || e2.sym)
tcc_error(invalid operation with label);
switch(op) {
case '*':
pe-v *= e2.v;
break;
case '/':
if (e2.v == 0) {
div_error:
tcc_error(division by zero);
}
pe-v /= e2.v;
break;
case '%':
if (e2.v == 0)
goto div_error;
pe-v %= e2.v;
break;
case TOK_SHL:
pe-v = e2.v;
break;
default:
case TOK_SAR:this case
after default what does it mean ??
pe-v = e2.v;
break;
}
}
}



On Wed, Jan 30, 2013 at 11:08 PM, Ivo van Poorten ivo...@gmail.com wrote:

 Error:

 $ ./configure --cc=tcc
 Binary  directory   /usr/local/bin
 TinyCC directory/usr/local/lib/tcc
 Library directory   /usr/local/lib
 Include directory   /usr/local/include
 Manual directory/usr/local/share/man
 Info directory  /usr/local/share/info
 Doc directory   /usr/local/share/doc/tcc
 Target root prefix
 Source path  /home/ivo/git/tinycc
 C compiler   tcc
 Target OSLinux
 CPU  x86
 Big Endian   no
 gprof enabledno
 cross compilers  no
 use libgcc   no
 Creating config.mak and config.h
 $ make
 [..snip..]
 gcc -c lib/bcheck.c -o bcheck.o -I. -I/home/ivo/git/tinycc -Wall -g -O2
 -mpreferred-stack-boundary=2 -m386 -malign-functions=0 -m32 cc1: error:
 unrecognized command line option -m386


 88888888

 diff --git a/Makefile b/Makefile
 index d257464..0333ebe 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -232,7 +232,7 @@ libtcc1.a : FORCE
  lib/%/libtcc1.a : FORCE $(PROGS_CROSS)
 @$(MAKE) -C lib cross TARGET=$*
  bcheck.o : lib/bcheck.c
 -   gcc -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
 +   $(CC) -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
  FORCE:

  # install

 ___
 Tinycc-devel mailing list
 Tinycc-devel@nongnu.org
 https://lists.nongnu.org/mailman/listinfo/tinycc-devel

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-30 Thread Domingo Alvarez Duarte 
Here is some that I found simple to make:

Author: mingodad mingo...@gmail.com  2013-01-31 01:17:50
Committer: mingodad mingo...@gmail.com  2013-01-31 01:17:50
Parent: 1b1e7ee1fd2f269872128dc5e8b830bd55dfa80c (Fix cross-compilation
out-of-tree build)
Branch: vio_patch
Follows: release_0_9_25
Precedes:

Alerts found using Coverity Scan

--- libtcc.c
---
index b0a9b1a..69d9e0d 100644
@@ -479,8 +479,7 @@ ST_FUNC void put_extern_sym2(Sym *sym, Section *section,
 case TOK_strlen:
 case TOK_strcpy:
 case TOK_alloca:
-strcpy(buf, __bound_);
-strcat(buf, name);
+snprintf(buf, sizeof(buf), __bound_%s, name);
 name = buf;
 break;
 }

--- tccelf.c
---
index a4dee19..479b2fa 100644
@@ -114,7 +114,7 @@ ST_FUNC int put_elf_sym(Section *s, uplong value,
unsigned long size,
 if (ELFW(ST_BIND)(info) != STB_LOCAL) {
 /* add another hashing entry */
 nbuckets = base[0];
-h = elf_hash(name) % nbuckets;
+h = name ? elf_hash(name) % nbuckets : 0;
 *ptr = base[2 + h];
 base[2 + h] = sym_index;
 base[1]++;
@@ -3052,7 +3052,7 @@ static int ld_add_file_list(TCCState *s1, const char
*cmd, int as_needed)
 ret = -1;
 goto lib_parse_error;
 }
-strcpy(libname, filename[1]);
+snprintf(libname, sizeof(libname), %s, filename[1]);
 libname_to_filename(s1, libname, filename);
 } else if (t != LD_TOK_NAME) {
 tcc_error_noabort(filename expected);

--- tccgen.c
---
index 4300403..e06a932 100644
@@ -361,6 +361,7 @@ void vpush64(int ty, unsigned long long v)
 CValue cval;
 CType ctype;
 ctype.t = ty;
+ctype.ref = 0;
 cval.ull = v;
 vsetc(ctype, VT_CONST, cval);
 }
@@ -1734,6 +1735,7 @@ ST_FUNC void gen_op(int op)
 }
 vswap();
 type1.t = t;
+type1.ref = 0;
 gen_cast(type1);
 vswap();
 /* special case for shifts and long long: we keep the shift as
@@ -2717,6 +2719,7 @@ static void struct_decl(CType *type, int u)
 v = anon_sym++;
 }
 type1.t = a;
+type1.ref = 0;
 /* we put an undefined size for struct/union */
 s = sym_push(v | SYM_STRUCT, type1, 0, -1);
 s-r = 0; /* default alignment is zero as gcc */
@@ -3396,6 +3399,7 @@ static void gfunc_param_typed(Sym *func, Sym *arg)
 /* default casting : only need to convert float to double */
 if ((vtop-type.t  VT_BTYPE) == VT_FLOAT) {
 type.t = VT_DOUBLE;
+type.ref = 0;
 gen_cast(type);
 }
 } else if (arg == NULL) {
@@ -3592,6 +3596,7 @@ ST_FUNC void unary(void)
 if ((vtop-r  (VT_VALMASK | VT_LVAL | VT_SYM)) == VT_CONST) {
 CType boolean;
 boolean.t = VT_BOOL;
+boolean.ref = 0;
 gen_cast(boolean);
 vtop-c.i = !vtop-c.i;
 } else if ((vtop-r  VT_VALMASK) == VT_CMP)
@@ -4101,6 +4106,7 @@ static void expr_cond(void)
 CType boolean;
 int c;
 boolean.t = VT_BOOL;
+boolean.ref = 0;
 vdup();
 gen_cast(boolean);
 c = vtop-c.i;
@@ -5574,7 +5580,7 @@ ST_FUNC void gen_inline_functions(void)
 str = fn-token_str;
 fn-sym = NULL;
 if (file)
-strcpy(file-filename, fn-filename);
+snprintf(file-filename, sizeof(file-filename), %s,
fn-filename);
 sym-r = VT_SYM | VT_CONST;
 sym-type.t = ~VT_INLINE;




On Thu, Jan 31, 2013 at 1:13 AM, Domingo Alvarez Duarte
mingo...@gmail.comwrote:

 Also on this function:

 CID 968150 (#1 of 1): Unchecked return value (CHECKED_RETURN)6.
 check_return: Calling function parse_btype(CType *, AttributeDef *)
 without checking return value (as is done elsewhere 7 out of 8 times).


 /* enum/struct/union declaration. u is either VT_ENUM or VT_STRUCT */
 static void struct_decl(CType *type, int u)
 {
 ...
 bit_pos = 0;
 offset = 0;
 while (tok != '}') {
 parse_btype(btype, ad);  no check made here
 ?
 while (1) {
 bit_size = -1;
 v = 0;
 type1 = btype;



 On Thu, Jan 31, 2013 at 1:07 AM, Domingo Alvarez Duarte 
 mingo...@gmail.com wrote:

 Also here :

 static void asm_expr_logic(TCCState *s1, ExprValue *pe)
 {
 int op;
 ExprValue e2;

 asm_expr_prod(s1, pe);

 for(;;) {
 op = tok;
 if (op != ''  op != '|'  op != '^')
 break;
 next();

Re: [Tinycc-devel] Small patch

2013-01-30 Thread Domingo Alvarez Duarte 
Also on this function:

CID 968150 (#1 of 1): Unchecked return value (CHECKED_RETURN)6.
check_return: Calling function parse_btype(CType *, AttributeDef *)
without checking return value (as is done elsewhere 7 out of 8 times).


/* enum/struct/union declaration. u is either VT_ENUM or VT_STRUCT */
static void struct_decl(CType *type, int u)
{
...
bit_pos = 0;
offset = 0;
while (tok != '}') {
parse_btype(btype, ad);  no check made here
?
while (1) {
bit_size = -1;
v = 0;
type1 = btype;



On Thu, Jan 31, 2013 at 1:07 AM, Domingo Alvarez Duarte
mingo...@gmail.comwrote:

 Also here :

 static void asm_expr_logic(TCCState *s1, ExprValue *pe)
 {
 int op;
 ExprValue e2;

 asm_expr_prod(s1, pe);

 for(;;) {
 op = tok;
 if (op != ''  op != '|'  op != '^')
 break;
 next();
 asm_expr_prod(s1, e2);

 if (pe-sym || e2.sym)
 tcc_error(invalid operation with label);
 switch(op) {
 case '':
 pe-v = e2.v;
 break;
 case '|':
 pe-v |= e2.v;
 break;
 default:
 case '^':  ///what this case
 after default mean 
 pe-v ^= e2.v;
 break;
 }
 }
 }



 On Thu, Jan 31, 2013 at 1:06 AM, Domingo Alvarez Duarte 
 mingo...@gmail.com wrote:

 Hello !
 I'm looking again on the reports done with Coverity Scan and found
 several things one of it is this:

 static void asm_expr_prod(TCCState *s1, ExprValue *pe)
 {
 int op;
 ExprValue e2;

 asm_expr_unary(s1, pe);
 for(;;) {
 op = tok;
 if (op != '*'  op != '/'  op != '%' 
 op != TOK_SHL  op != TOK_SAR)
 break;
 next();
 asm_expr_unary(s1, e2);
 if (pe-sym || e2.sym)
 tcc_error(invalid operation with label);
 switch(op) {
 case '*':
 pe-v *= e2.v;
 break;
 case '/':
 if (e2.v == 0) {
 div_error:
 tcc_error(division by zero);
 }
 pe-v /= e2.v;
 break;
 case '%':
 if (e2.v == 0)
 goto div_error;
 pe-v %= e2.v;
 break;
 case TOK_SHL:
 pe-v = e2.v;
 break;
 default:
 case TOK_SAR:this
 case after default what does it mean ??
 pe-v = e2.v;
 break;
 }
 }
 }



 On Wed, Jan 30, 2013 at 11:08 PM, Ivo van Poorten ivo...@gmail.comwrote:

 Error:

 $ ./configure --cc=tcc
 Binary  directory   /usr/local/bin
 TinyCC directory/usr/local/lib/tcc
 Library directory   /usr/local/lib
 Include directory   /usr/local/include
 Manual directory/usr/local/share/man
 Info directory  /usr/local/share/info
 Doc directory   /usr/local/share/doc/tcc
 Target root prefix
 Source path  /home/ivo/git/tinycc
 C compiler   tcc
 Target OSLinux
 CPU  x86
 Big Endian   no
 gprof enabledno
 cross compilers  no
 use libgcc   no
 Creating config.mak and config.h
 $ make
 [..snip..]
 gcc -c lib/bcheck.c -o bcheck.o -I. -I/home/ivo/git/tinycc -Wall -g -O2
 -mpreferred-stack-boundary=2 -m386 -malign-functions=0 -m32 cc1: error:
 unrecognized command line option -m386


 88888888

 diff --git a/Makefile b/Makefile
 index d257464..0333ebe 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -232,7 +232,7 @@ libtcc1.a : FORCE
  lib/%/libtcc1.a : FORCE $(PROGS_CROSS)
 @$(MAKE) -C lib cross TARGET=$*
  bcheck.o : lib/bcheck.c
 -   gcc -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
 +   $(CC) -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
  FORCE:

  # install

 ___
 Tinycc-devel mailing list
 Tinycc-devel@nongnu.org
 https://lists.nongnu.org/mailman/listinfo/tinycc-devel




___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Small patch

2013-01-30 Thread Domingo Alvarez Duarte 
Also here :

static void asm_expr_logic(TCCState *s1, ExprValue *pe)
{
int op;
ExprValue e2;

asm_expr_prod(s1, pe);
for(;;) {
op = tok;
if (op != ''  op != '|'  op != '^')
break;
next();
asm_expr_prod(s1, e2);
if (pe-sym || e2.sym)
tcc_error(invalid operation with label);
switch(op) {
case '':
pe-v = e2.v;
break;
case '|':
pe-v |= e2.v;
break;
default:
case '^':  ///what this case
after default mean 
pe-v ^= e2.v;
break;
}
}
}



On Thu, Jan 31, 2013 at 1:06 AM, Domingo Alvarez Duarte
mingo...@gmail.comwrote:

 Hello !
 I'm looking again on the reports done with Coverity Scan and found several
 things one of it is this:

 static void asm_expr_prod(TCCState *s1, ExprValue *pe)
 {
 int op;
 ExprValue e2;

 asm_expr_unary(s1, pe);
 for(;;) {
 op = tok;
 if (op != '*'  op != '/'  op != '%' 
 op != TOK_SHL  op != TOK_SAR)
 break;
 next();
 asm_expr_unary(s1, e2);
 if (pe-sym || e2.sym)
 tcc_error(invalid operation with label);
 switch(op) {
 case '*':
 pe-v *= e2.v;
 break;
 case '/':
 if (e2.v == 0) {
 div_error:
 tcc_error(division by zero);
 }
 pe-v /= e2.v;
 break;
 case '%':
 if (e2.v == 0)
 goto div_error;
 pe-v %= e2.v;
 break;
 case TOK_SHL:
 pe-v = e2.v;
 break;
 default:
 case TOK_SAR:this case
 after default what does it mean ??
 pe-v = e2.v;
 break;
 }
 }
 }



 On Wed, Jan 30, 2013 at 11:08 PM, Ivo van Poorten ivo...@gmail.comwrote:

 Error:

 $ ./configure --cc=tcc
 Binary  directory   /usr/local/bin
 TinyCC directory/usr/local/lib/tcc
 Library directory   /usr/local/lib
 Include directory   /usr/local/include
 Manual directory/usr/local/share/man
 Info directory  /usr/local/share/info
 Doc directory   /usr/local/share/doc/tcc
 Target root prefix
 Source path  /home/ivo/git/tinycc
 C compiler   tcc
 Target OSLinux
 CPU  x86
 Big Endian   no
 gprof enabledno
 cross compilers  no
 use libgcc   no
 Creating config.mak and config.h
 $ make
 [..snip..]
 gcc -c lib/bcheck.c -o bcheck.o -I. -I/home/ivo/git/tinycc -Wall -g -O2
 -mpreferred-stack-boundary=2 -m386 -malign-functions=0 -m32 cc1: error:
 unrecognized command line option -m386


 88888888

 diff --git a/Makefile b/Makefile
 index d257464..0333ebe 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -232,7 +232,7 @@ libtcc1.a : FORCE
  lib/%/libtcc1.a : FORCE $(PROGS_CROSS)
 @$(MAKE) -C lib cross TARGET=$*
  bcheck.o : lib/bcheck.c
 -   gcc -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
 +   $(CC) -c $ -o $@ $(CPPFLAGS) $(CFLAGS)
  FORCE:

  # install

 ___
 Tinycc-devel mailing list
 Tinycc-devel@nongnu.org
 https://lists.nongnu.org/mailman/listinfo/tinycc-devel



___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel