bug в gcc или я чего-то не понимаю?

tokuchu

Компилирую zfsonlinux и получаю вот такое:
  CC [M]  /home/user/soft/spl/module/spl/../../module/spl/spl-module.o
/home/user/soft/spl/module/spl/../../module/spl/spl-module.c: In function ‘__ddi_create_minor_node’:
/home/user/soft/spl/module/spl/../../module/spl/spl-module.c:124:2: error: assignment of read-only location ‘*fops’
/home/user/soft/spl/module/spl/../../module/spl/spl-module.c:126:3: error: assignment of read-only location ‘*fops’
/home/user/soft/spl/module/spl/../../module/spl/spl-module.c:128:3: error: assignment of read-only location ‘*fops’
make[5]: *** [/home/user/soft/spl/module/spl/../../module/spl/spl-module.o] Error 1
make[4]: *** [/home/user/soft/spl/module/spl] Error 2
make[3]: *** [_module_/home/user/soft/spl/module] Error 2

При этом код там такой. И я не вижу почему оно read-only.

92 int
93 __ddi_create_minor_node(dev_info_t *di, char *name, int spec_type,
94 minor_t minor_num, char *node_type,
95 int flags, struct module *mod)
96 {
97 struct cdev *cdev;
98 struct cb_ops *cb_ops;
99 struct file_operations *fops;
100 int rc;
101 SENTRY;
...
123 /* Setup the fops to cb_ops mapping */
124 fops->owner = mod;
125 if (cb_ops->cb_ioctl) {
126 fops->unlocked_ioctl = mod_generic_unlocked_ioctl;
127 #ifdef CONFIG_COMPAT
128 fops->compat_ioctl = mod_generic_compat_ioctl;
129 #endif
130 }

kataich

Ты нашел, чему *fops в итоге равно?
Из твоего отрывка непонятно.

tokuchu

Ты нашел, чему *fops в итоге равно?
Из твоего отрывка непонятно.
А он это в compile-time определяет чему оно может быть равно?

Maurog

я чего-то не понимаю
хреновый у тебя баг репорт :grin:
ни кода нормального, ни версий компилятора
http://comments.gmane.org/gmane.linux.file-systems.zfs.user/...

tokuchu

хреновый у тебя баг репорт :grin:
ни кода нормального, ни версий компилятора
А что ты от кода ещё хочешь? Могу ссылку дать на весь. :)
Версию забыл написать, да. gcc-4.5.3 в gentoo. Кажется ещё 4.4.5 там же пробовал.
http://comments.gmane.org/gmane.linux.file-systems.zfs.user/...
Там чего-то ответа на вопрос нету. :)

kataich

SENTRY - макрос?
Покажи, плиз, что со 101 до 123 строчки происходит.

procenkotanya

для нормального багрепорта нужен .i от make /home/user/soft/spl/module/spl/../../module/spl/spl-module.i и флаги компилятора

tokuchu

SENTRY - макрос?
Покажи, плиз, что со 101 до 123 строчки происходит.
Вот исходники:
http://github.com/zfsonlinux/spl/blob/master/module/spl/spl...
SENTRY нашёл в include/spl-debug.h, вроде безобидный макрос для дебага.

tokuchu

для нормального багрепорта нужен .i от make /home/user/soft/spl/module/spl/../../module/spl/spl-module.i и флаги компилятора
Ну я вообще хз повод ли это вообще для багрепорта. И если да, то тогда уже надо будет разобраться что там и кому надо. :)

kataich

Ага, уже нашел.

kataich

Воспроизвести нет возможности, а глазами не понимаю, к сожалению.
Не может же это быть из-за того, что после kzalloc (void*) в (file_operations*)
явно не преобразовали.
P.S. Интересно, а будет ли ругаться, если во всех таких местах заменить
fops->* на cdev->ops->*

tokuchu

Не может же это быть из-за того, что после kzalloc (void*) в (file_operations*)
явно не преобразовали.
Ну так это тогда там бы и ругнулось бы?

tokuchu

P.S. Интересно, а будет ли ругаться, если во всех таких местах заменить
fops->* на cdev->ops->*
Поменял в одной строчке. Теперь сказало:
error: assignment of read-only location ‘*cdev->ops’
:)

kataich

Что-нибудь накопал?

kataich

У меня нет возможности проверять на более свежем ядре, поэтому у меня в других местах всё валится, но именно spl-module.c скомпилировался нормально.

# uname -a
Linux test 2.6.17-11-generic SMP Thu Feb 1 19:52:28 UTC 2007 i686 GNU/Linux

# gcc --version
gcc (GCC) 4.1.2 (Ubuntu 4.1.2-0ubuntu4)
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

# make
...
CC [M] /home/test/test/spl/module/spl/../../module/spl/spl-module.o
CC [M] /home/test/test/spl/module/spl/../../module/spl/spl-generic.o
...

ppplva

i или не считается.

Anturag

 
$ gcc -c -o test.o test.c 
test.c: In function ‘wtf’:
test.c:7:5: error: assignment of member ‘owner’ in read-only object
$ cat test.c
const struct {
void *owner;
} __fops = { .owner = (void *)0, }, *fops = &__fops;

void wtf(void)
{
fops->owner = 0;
}

Серьёзный баг в гцц...

tokuchu

const struct
Ну там же не const?
Сейчас посмотрел объявление struct file_operations в linux/fs.h — там есть какая-то приписка в конце __do_const. Которая вроде как макрос и как я понял в gcc4 разворачивается в __attribute__do_const которое мне уже не совсем понятно.

tokuchu

.i или не считается.
Тебе весь выдать? Или этого хватит:

struct file_operations {
struct module *owner;
loff_t (*llseek) (struct file *, loff_t, int);
ssize_t (*read) (struct file *, char *, size_t, loff_t *);
ssize_t (*write) (struct file *, const char *, size_t, loff_t *);
ssize_t (*aio_read) (struct kiocb *, const struct iovec *, unsigned long, loff_t);
ssize_t (*aio_write) (struct kiocb *, const struct iovec *, unsigned long, loff_t);
int (*readdir) (struct file *, void *, filldir_t);
unsigned int (*poll) (struct file *, struct poll_table_struct *);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
int (*open) (struct inode *, struct file *);
int (*flush) (struct file *, fl_owner_t id);
int (*release) (struct inode *, struct file *);
int (*fsync) (struct file *, int datasync);
int (*aio_fsync) (struct kiocb *, int datasync);
int (*fasync) (int, struct file *, int);
int (*lock) (struct file *, int, struct file_lock *);
ssize_t (*sendpage) (struct file *, struct page *, int, size_t, loff_t *, int);
unsigned long (*get_unmapped_areastruct file *, unsigned long, unsigned long, unsigned long, unsigned long);
int (*check_flagsint);
int (*flock) (struct file *, int, struct file_lock *);
ssize_t (*splice_writestruct pipe_inode_info *, struct file *, loff_t *, size_t, unsigned int);
ssize_t (*splice_readstruct file *, loff_t *, struct pipe_inode_info *, size_t, unsigned int);
int (*setleasestruct file *, long, struct file_lock **);
long (*fallocatestruct file *file, int mode, loff_t offset,
loff_t len);
} __attribute__do_const;
typedef struct file_operations __attribute__no_const file_operations_no_const;
...
int
__ddi_create_minor_node(dev_info_t *di, char *name, int spec_type,
minor_t minor_num, char *node_type,
int flags, struct module *mod)
{
struct cdev *cdev;
struct cb_ops *cb_ops;
struct file_operations *fops;
int rc;
void)0);

void)0);
void)0);
void)0);

fops = ({ void *___retval; intoverflow_t ___x = (intoverflow_t)sizeof(struct file_operations); if { int __ret_warn_on = !(___x > (~0UL; if (__builtin_expect(!(__ret_warn_on 0 warn_slowpath_fmt("/home/user/soft/spl/module/spl/../../module/spl/spl-module.c", 107, "kzalloc size overflow\n"); __builtin_expect(!(__ret_warn_on 0); } ___retval = void *)0); else ___retval = kzallocsize_t)___x, gfp_t)0x10u) | gfp_t)0x40u) | gfp_t)0x80u; ___retval; });
if (fops == void *)0
return (-1);

cdev = cdev_alloc;
if (cdev == void *)0 {
kfree(fops);
return (-1);
}

cdev->ops = fops;

mutex_lock(&(&di->di_lock)->m);
cb_ops = di->di_ops->devo_cb_ops;
void)0);



fops->owner = mod;
if (cb_ops->cb_ioctl) {
fops->unlocked_ioctl = mod_generic_unlocked_ioctl;

fops->compat_ioctl = mod_generic_compat_ioctl;

}

tokuchu

Которая вроде как макрос и как я понял в gcc4 разворачивается в __attribute__do_const которое мне уже не совсем понятно.
Это похоже ещё появляется после hardened патчей (в gentoo). Но всё равно я точно не знаю что всё это делает.

procenkotanya

Атрибут добавляется неким constify-plugin для gcc, так что баг не в гцц.

tokuchu

Атрибут добавляется неким constify-plugin для gcc, так что баг не в гцц.
А это баг или всё-таки фича этого плагина? :)

tokuchu

Наверное пойду постить баг в gentoo-шной багзиле. Там расскажут. :)

vall

кто-то помешался на секьюрити.
хотя я не вижу смысла делать не константный struct file_operations,
это явно какой-то гавнокод если ему нужно их динамически заполнять.

tokuchu

кто-то помешался на секьюрити.
Ну это в gentoo-hardened патчи на ядро. То ли grsecurity, то ли pax.
хотя я не вижу смысла делать не константный struct file_operations,
это явно какой-то гавнокод если ему нужно их динамически заполнять.
Ссылка на исходник есть, можешь патч предложить. :)

Anturag

const struct

Ну там же не const?

Я тебе наглядно показал пример того, что компилятор считает, что изменять что-либо в .rodata не комильфо. Причём здесь gcc, если проблема в коде, на основе которого gcc пихает fops в .rodata? В провокативной манере показал тебе, что из "bug в gcc или чего-то не понимаю" надо выбрать второе, а на данный момент уже "чего-то не понимал".
То, что нужно было процитировать из кода, в твоём заглавном посте отсутствует, тебе на это указали и до моего поста-подсказки, хорошо, что ты после него привёл всё необходимое для нетелепатов.

Anturag

the constify plugin make ops structures and those marked with the (newly introduced) do_const attribute read-only at compile time and consequently at runtime as well (non-static allocations will be flagged by the compiler and the source has to be patched to use a writable type marked with the (newly introduced, again) no_const attribute).

Читаем LWN.

xronik111

Кстати, тред отличный показатель того, насколько сосут самопридуманные расширения языка.

tokuchu

Я тебе наглядно показал пример того, что компилятор считает, что изменять что-либо в .rodata не комильфо.
Ну это как-то совсем по капитански. А твой код, согласись, достаточно отличается от того, что там в исходниках написано.
Причём здесь gcc, если проблема в коде, на основе которого gcc пихает fops в .rodata? В провокативной манере показал тебе, что из "bug в gcc или чего-то не понимаю" надо выбрать второе, а на данный момент уже "чего-то не понимал".
Это не провокативная манера была, а неочевидная. Т.к. то, что он считает данные рид-онли было понятно, а вопрос был в том почему он так делает.

vall

Кстати, тред отличный показатель того, насколько сосут самопридуманные расширения языка.
расширение на самом деле неплохое, диагностика плохая.

ppplva

Как плохая диагностика? Английским по белому сказано - присваивать нельзя потому что локейшен рид-онли. Смотришь на локейшен (что здесь сделали бы за 5 минут, если бы автор догадался запостить .i, или хотя бы объявление fops) - видишь атрибут с говорящим названием.
Если для кого-то название не совсем говорящее (или плохо, невнятно говорящее) - вбиваешь его в гугл, там по нескольким первым ссылкам объясняют так что и ребенок поймет.

tokuchu

Смотришь на локейшен (что здесь сделали бы за 5 минут, если бы автор догадался запостить .i, или хотя бы объявление fops) - видишь атрибут с говорящим названием.
Ну-ну. Я же запостил .i — там объявление fops абсолютно точно такое же:
 struct file_operations *fops;

ppplva

Описание, лол. Definition.
Не все инструкции нужно понимать дословно.
} __attribute__do_const; 

ppplva

Тьфу блин, наоборот, type declaration.

vall

вот именно, они растянули смысл существующей диагностики.
лучше бы они метили отдельные поля структур, и добавить какой-нить способ подавить ворнинг.
в ядре полно структур которые рид-онли только наполовину.

tokuchu

Не все инструкции нужно понимать дословно.
Тогда нужно не про fops писать было, а про file_operations.

xronik111

Идея наверняка хорошая, я не про это, а про капитанскую мысль, что любое расширение языка есть бег по лезвию бритвы. Много званных, но мало избранных — нужно быть сильно language lawyer'ом, чтобы четко представлять себе последствия.
Здесь я даже готов допустить, что авторы их представляли, раз ввели диагностику, но уже пользователи имеют проблемы, ибо происходящее не укладывается в их картину мира Си. Причем грин-то еще не лох, а если взять обычного программиста на Си, то гарантированно будет бугога. Поэтому из gcc все время вычищают эти расширения, и новых стараются не добавлять, слава яйцам, что хоть для этого можно использовать плагины.
Да чего далеко ходить, вон небезызвестный Ульрих Дреппер недавно в рассылке спрашивал, а что это у него С++-код не компилируется, а оказалось, что было недокументированные расширение, убитое еще со времени 4.2, когда затягивали компилятор на соответствие стандарту плюсов, и о чем даже есть текст в документации по портированию кода.
Оставить комментарий
Имя или ник:
Комментарий: