Hi Sun and all,

Resent the code review request.
It seems no document to write clear when can we use the base and ofst.
There are hidden, internal, protected, default to set the visibility. There
is a simple case to see how gcc works:
int hidden __attribute__ ((visibility("hidden")))=3;
int internal __attribute__ ((visibility("internal")))=3;
int protected __attribute__ ((visibility("protected")))=3;
int preemitable __attribute__ ((visibility("default")))=3;
static int global=4;
int main()
{
    hidden=4;
    internal=4;
    protected=6;
    preemitable=4;
    global=4;

    return 0;
}
The output of ".s" shows that all defined symbols *do not* use  base symbol
and ofst.
Current open64 code shows only under PIC shared non export local symbol do
not use base and ofst. This does not work for bug848 case, it need exclude
"preemitable symbols" when generate non pic code. so what about other
symbol types under non pic?  hidden, internal, protected  symbols canbe
seen controlled by linker, we'd better not use base and ofst to represent
them either. So in my revised patch, no matter pic or nonpic only
exported_local symbols canbe emit with base and ofst.
    710   while( (ST_base(base) != base  )
    711          && (ST_sclass(base) != SCLASS_TEXT)
*    712          && !((Gen_PIC_Shared || Gen_PIC_Call_Shared) &&
!ST_is_export_local(base))*
    713 #ifdef KEY
    714          && !ST_is_weak_symbol(base)
    715          && !ST_is_thread_local(base)
    716 #endif // KEY
    717          )
    718   {
    719       tofst += ST_ofst(base);
    720       base = ST_base(base);
    721   }

bug848_1 Patch:
Index: /export/home/zhuqing/trunk/osprey/be/com/stblock.cxx
===================================================================
--- /export/home/zhuqing/trunk/osprey/be/com/stblock.cxx        (revision
3827)
+++ /export/home/zhuqing/trunk/osprey/be/com/stblock.cxx        (working
copy)
@@ -692,8 +692,8 @@
   /* 1. For text symbols, don't use the base_sym and base_ofst because
    *    they may not have been set yet. This happens for a forward
    *    reference to a text address.
-   * 2. For preemptible symbols, we cannot use the base where the symbol
-   *    is allocated since it could be preempted.
+   * 2. For none local symbols we cannot use the base, since they can
+   *    be seen and controlled by linker
    */
 #ifdef KEY
   /* 3. For weak symbol, we cannot use the base where the symbol
@@ -709,7 +709,7 @@

   while( (ST_base(base) != base  )
         && (ST_sclass(base) != SCLASS_TEXT)
-        && !((Gen_PIC_Shared || Gen_PIC_Call_Shared) &&
!ST_is_export_local(base))
+        && ST_is_export_local(base)
 #ifdef KEY
         && !ST_is_weak_symbol(base)
         && !ST_is_thread_local(base)

Thanks
zhuqing
---------- Forwarded message ----------
From: 朱庆 <zqing1...@gmail.com>
Date: 2011/11/11
Subject: [Open64-devel] Code Review request for bug848 [CG]
To: open64-devel@lists.sourceforge.net


Hi all,

Can gatekeeper help review for a fix for bug848?
https://bugs.open64.net/show_bug.cgi?id=848

Case:
$cat timer.c
#include <stdio.h>
extern unsigned long long __attribute__((section(".data"))) jiffies_64;
extern unsigned long volatile __attribute__((section(".data"))) jiffies;
unsigned long volatile __attribute__((section(".data"))) jiffies=100;
unsigned long long  jiffies_64 __attribute__((__aligned__((1 << (6))))) =
((unsigned long)(unsigned int) (-300*1000));
void do_timer(unsigned long ticks)
{
       jiffies_64 += ticks;
}
int main()
{
do_timer(1);
printf("%d\n", jiffies_64);
printf("%d\n", jiffies);
}

$cat a.lds:
iffies_64 = jiffies;

The problem is when link with "a.lds", the output of open64 is wrong.
The output of jiffies_64 and jiffies should be the same according to
the linker script.
opencc a.lds timer.c -O0 -keep
./a.out
-299999
100

when compile with gcc
./a.out
101
101

The root reason is in open64, the preemitable symbol is addressed
through base and offset.
the output of timer.s:
 #   3  extern unsigned long volatile __attribute__((section(".data")))
jiffies;
 #   4  unsigned long volatile __attribute__((section(".data")))
jiffies=100;
 #   5  unsigned long long  jiffies_64 __attribute__((__aligned__((1 <<
(6)))))
= ((unsigned long)(unsigned int) (-300*1000));
 #   6  void do_timer(unsigned long ticks)
 #   7  {
.LBB1_do_timer:
       pushq %rbp                      #
       movq %rsp,%rbp                  #
       addq $-32,%rsp                  #
       movq %rdi,-16(%rbp)             # ticks
       .loc    1       8       0
 #   8          jiffies_64 += ticks;
       movq -16(%rbp),%rdi             # ticks
       movq .data(%rip),%rax           #
       addq %rdi,%rax                  #
       movq %rax,.data(%rip)           #
       leave                           #
       ret                             #
.LDWend_do_timer:
       .size do_timer, .LDWend_do_timer-do_timer

symbol jiffies_64 is represent through .data(%rip), but not
 jiffies_64(%rip),
so when in the linker script reset the address of jiffies_64 does not work.

The patch attached at bug848_1.patch.  There is a regression caused by
bug848_1.patch, the case is:

extern void abort (void);

volatile int out = 1;
volatile int a = 2;
volatile int b = 4;
volatile int c = 8;
volatile int d = 16;
volatile int e = 32;
volatile int f = 64;

int
main ()
{
 asm volatile ("xorl %%eax, %%eax      \n\t"
               "xorl %%esi, %%esi      \n\t"
               "addl %1, %0            \n\t"
               "addl %2, %0            \n\t"
               "addl %3, %0            \n\t"
               "addl %4, %0            \n\t"
               "addl %5, %0            \n\t"
               "addl %6, %0"
               : "+r" (out)
               : "r" (a), "r" (b), "r" (c), "g" (d), "g" (e), "g" (f)
               : "%eax", "%esi");

 if (out != 127)
   abort ();

 return 0;
}

The cause is that for asm string,  there is following code in fun
"Modify_Asm_String":
   ST* st = TN_var(tn);
   Base_Symbol_And_Offset( st, &base_st, &base_ofst );
 Base_Symbol_And_Offset does not consider whether shoud use
base-offset or not. So I change the call to
"Base_Symbol_And_Offset_For_Addressing". see bug848_2.patch

Thanks
zhuqing
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure 
contains a definitive record of customers, application performance, 
security threats, fraudulent activity, and more. Splunk takes this 
data and makes sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-novd2d
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to